Skip to content

Protected containers: Align actor for feature flag

Gerardo Navarro requested to merge gitlab-community/gitlab:480848-gerardo-navarro-protected-containers-use-consistent-actor-when-checking-for-feature-flag-container-registry-protected-containers into master
  • Please check this box if this contribution uses AI-generated content (including content generated by GitLab Duo features) as outlined in the GitLab DCO & CLA

What does this MR do and why?

As explained in the related issue, the feature flag (FF) :container_registry_protected_containers needs to be applied to a consistent actor type. This is prerequisite for the rollout procedure of the FF as described here.

At the moment, the feature flag is applied to actor types Project and Group. In order to align the actor types, this MR contains the changes for the approach outlined by @10io. This approach uses the method #root_namespace to get a consistent Namespace actor object. In case of the (original) actor type Project, the #root_namespace will be either a Group (will be the root parent group and also considered a Namespace object) or UserNamespace. In case of the actor type Group, use the method #root_ancestor in order to return the root parent group of given group.

Advantages and disadvantages

The advantage of this approach is that we can keep a single feature flag :container_registry_protected_containers to control all aspects of the feature. The disadvantage is that handling different actor types makes the code and testing more complex (and maybe more errorprone although this was one goal of the refactoring).

Decision: In another discussion thread, we discussed / decided that it is better to have a single feature flag to control all aspects even if the complexity of the implmenetation increases.

Implementation plan

NOTE: To finalize this MR, it is necessary to apply the same concept to the following occurrences of the feature flag:

  • app/controllers/groups/registry/repositories_controller.rb: push_frontend_feature_flag(:container_registry_protected_containers, group) # <- type Group
  • app/controllers/projects/registry/repositories_controller.rb: push_frontend_feature_flag(:container_registry_protected_containers, project) # <- type Project
  • app/controllers/projects/settings/packages_and_registries_controller.rb: before_action :set_feature_flag_container_registry_protected_containers, only: :show
  • app/controllers/projects/settings/packages_and_registries_controller.rb: push_frontend_feature_flag(:container_registry_protected_containers, project)
  • app/graphql/mutations/container_registry/protection/rule/create.rb: if Feature.disabled?(:container_registry_protected_containers, project)
  • app/graphql/mutations/container_registry/protection/rule/delete.rb: if Feature.disabled?(:container_registry_protected_containers, container_registry_protection_rule.project)
  • app/graphql/mutations/container_registry/protection/rule/update.rb: if Feature.disabled?(:container_registry_protected_containers, container_registry_protection_rule.project)
  • app/graphql/resolvers/project_container_registry_protection_rules_resolver.rb: return [] if Feature.disabled?(:container_registry_protected_containers, project)
  • app/graphql/types/container_repository_type.rb: return false if Feature.disabled?(:container_registry_protected_containers, object.project)
  • app/services/auth/container_registry_authentication_service.rb: return false if Feature.disabled?(:container_registry_protected_containers, project)
  • lib/api/project_container_registry_protection_rules.rb: if Feature.disabled?(:container_registry_protected_containers, user_project)

NOTE: In another MR, I tried out another (maybe easier) approach to solve the different actor type.

🛠 with ❤ at Siemens

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

MR Checklist (@gerardo-navarro)

Screenshots or screen recordings

How to set up and validate locally

Related to #480848 (closed)

Edited by Gerardo Navarro

Merge request reports