From dcb04dc818e0e0b0e836ac1ecb25cf64038e64a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Wed, 8 Jan 2025 10:59:12 +0100 Subject: [PATCH 1/3] add json and yaml basic validator into ci --- .github/workflows/json-yaml-validation.yml | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/json-yaml-validation.yml diff --git a/.github/workflows/json-yaml-validation.yml b/.github/workflows/json-yaml-validation.yml new file mode 100644 index 000000000..c334526af --- /dev/null +++ b/.github/workflows/json-yaml-validation.yml @@ -0,0 +1,23 @@ +name: json-yaml-validate +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + +permissions: + contents: read + pull-requests: write # enable write permissions for pull request comments + +jobs: + json-yaml-validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: json-yaml-validate + id: json-yaml-validate + uses: GrantBirki/json-yaml-validate@v3.2.1 # https://github.com/GrantBirki/json-yaml-validate/ + with: + comment: "true" # enable comment mode From 79a3230a5393aa121440bd383edae8ea33197f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petra=20C=CC=8Ci=CC=81halova=CC=81?= Date: Wed, 8 Jan 2025 11:08:40 +0100 Subject: [PATCH 2/3] fix yaml files - update key for resource requests - use ${} for env vars placeholders to be not confused with yaml directives --- openshift/rbac-template.yaml | 2 +- openshift/redis.yaml | 2 +- scripts/ephemeral/config_template.yaml | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openshift/rbac-template.yaml b/openshift/rbac-template.yaml index ca2146ef0..de55cdaae 100644 --- a/openshift/rbac-template.yaml +++ b/openshift/rbac-template.yaml @@ -514,7 +514,7 @@ objects: selector: name: rbac-pgsql resources: - limits: + requests: memory: ${MEMORY_REQUEST} limits: memory: ${MEMORY_LIMIT} diff --git a/openshift/redis.yaml b/openshift/redis.yaml index 4819fd564..902c36e95 100644 --- a/openshift/redis.yaml +++ b/openshift/redis.yaml @@ -53,7 +53,7 @@ objects: selector: name: ${NAME}-redis resources: - limits: + requests: memory: ${MEMORY_REQUEST} limits: memory: ${MEMORY_LIMIT} diff --git a/scripts/ephemeral/config_template.yaml b/scripts/ephemeral/config_template.yaml index c0e0ff6c4..9044bbbdd 100644 --- a/scripts/ephemeral/config_template.yaml +++ b/scripts/ephemeral/config_template.yaml @@ -13,11 +13,11 @@ apps: components: - name: rbac host: local - repo: %REPO% + repo: $(REPO} path: /deploy/rbac-clowdapp.yml parameters: - IMAGE_TAG: %IMAGE_TAG% - IMAGE: %IMAGE% + IMAGE_TAG: ${IMAGE_TAG} + IMAGE: ${IMAGE} CW_NULL_WORKAROUND: false MIN_SCHEDULER_REPLICAS: 1 CELERY_WORKER_MEMORY_LIMIT: '2Gi' From 19b59fb9a549f4565875697f44a3b700d532c5f9 Mon Sep 17 00:00:00 2001 From: Jay Z Date: Tue, 14 Jan 2025 21:26:48 +0800 Subject: [PATCH 3/3] [RHCLOUD-36626] System roles are protected from update (#1423) * System roles are protected from update * Address feedbacks --------- Co-authored-by: Libor Pichler --- rbac/management/role/serializer.py | 70 +++++++++++++----------------- tests/management/role/test_view.py | 13 +++--- 2 files changed, 36 insertions(+), 47 deletions(-) diff --git a/rbac/management/role/serializer.py b/rbac/management/role/serializer.py index cb4bf515a..1cd0dd190 100644 --- a/rbac/management/role/serializer.py +++ b/rbac/management/role/serializer.py @@ -144,10 +144,8 @@ def update(self, instance, validated_data): """Update the role object in the database.""" access_list = validated_data.pop("access") tenant = self.context["request"].tenant - role_name = instance.name - update_data = validate_role_update(instance, validated_data) - instance = update_role(role_name, update_data, tenant) + instance = update_role(instance, validated_data) create_access_for_role(instance, access_list, tenant) @@ -161,6 +159,15 @@ def get_external_tenant(self, obj): """Get the external tenant name if it's from an external tenant.""" return obj.external_tenant_name() + def validate(self, data): + """Validate the input data of role.""" + if self.instance and self.instance.system: + key = "role.update" + message = "System roles may not be updated." + error = {key: [_(message)]} + raise serializers.ValidationError(error) + return super().validate(data) + class RoleMinimumSerializer(SerializerCreateOverrideMixin, serializers.ModelSerializer): """Serializer for the Role model that doesn't return access info.""" @@ -308,13 +315,18 @@ class RolePatchSerializer(RoleSerializer): def update(self, instance, validated_data): """Patch the role object.""" - tenant = self.context["request"].tenant - role_name = instance.name - update_data = validate_role_update(instance, validated_data) - - instance = update_role(role_name, update_data, tenant, clear_access=False) + instance = update_role(instance, validated_data, clear_access=False) return instance + def validate(self, data): + """Validate the input data of patching role.""" + if self.instance.system: + key = "role.update" + message = "System roles may not be updated." + error = {key: [_(message)]} + raise serializers.ValidationError(error) + return super().validate(data) + class BindingMappingSerializer(serializers.ModelSerializer): """Serializer for the binding mapping.""" @@ -382,43 +394,19 @@ def create_access_for_role(role, access_list, tenant): ResourceDefinition.objects.create(**resource_def_item, access=access_obj, tenant=tenant) -def validate_role_update(instance, validated_data): - """Validate if role could be updated.""" - if instance.system: - key = "role.update" - message = "System roles may not be updated." - error = {key: [_(message)]} - raise serializers.ValidationError(error) - updated_name = validated_data.get("name", instance.name) - updated_display_name = validated_data.get("display_name", instance.display_name) - updated_description = validated_data.get("description", instance.description) - - return { - "updated_name": updated_name, - "updated_display_name": updated_display_name, - "updated_description": updated_description, - } - - -def update_role(role_name, update_data, tenant, clear_access=True): +def update_role(instance, validated_data, clear_access=True): """Update role attribute.""" - role = Role.objects.get(name=role_name, tenant=tenant) - update_fields = [] - if "updated_name" in update_data: - role.name = update_data["updated_name"] - update_fields.append("name") - if "updated_display_name" in update_data: - role.display_name = update_data["updated_display_name"] - update_fields.append("display_name") - if "updated_description" in update_data: - role.description = update_data["updated_description"] - update_fields.append("description") + for field_name in ["name", "display_name", "description"]: + if field_name not in validated_data: + continue + setattr(instance, field_name, validated_data[field_name]) + update_fields.append(field_name) - role.save(update_fields=update_fields) + instance.save(update_fields=update_fields) if clear_access: - role.access.all().delete() + instance.access.all().delete() - return role + return instance diff --git a/tests/management/role/test_view.py b/tests/management/role/test_view.py index c0c10c1ea..7f4477c9f 100644 --- a/tests/management/role/test_view.py +++ b/tests/management/role/test_view.py @@ -176,16 +176,16 @@ def setUp(self): self.groupTwo.policies.add(self.policyTwo) self.groupTwo.save() - self.adminRole = Role(**admin_def_role_config, tenant=self.tenant) - self.adminRole.save() - - self.platformAdminRole = Role(**platform_admin_def_role_config, tenant=self.tenant) - self.platformAdminRole.save() - self.public_tenant = Tenant.objects.get(tenant_name="public") self.sysPubRole = Role(**sys_pub_role_config, tenant=self.public_tenant) self.sysPubRole.save() + self.adminRole = Role(**admin_def_role_config, tenant=self.public_tenant) + self.adminRole.save() + + self.platformAdminRole = Role(**platform_admin_def_role_config, tenant=self.public_tenant) + self.platformAdminRole.save() + self.sysRole = Role(**sys_role_config, tenant=self.public_tenant) self.sysRole.save() @@ -1684,6 +1684,7 @@ def test_update_admin_default_role(self): test_data = {"name": "role_name", "display_name": "role_display", "access": access_data} response = client.put(url, test_data, format="json", **self.headers) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data["errors"][0]["detail"], "System roles may not be updated.") def test_delete_default_role(self): """Test that default roles are protected from deletion"""