[go: up one dir, main page]
Skip to content

Fix Import/Export new attributes detection spec

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

The Import/Export has a spec that was designed to identify changes, urging developers to consider adding new attributes added to already exported tables to the import_export.yml file. The specs work by failing the CI Pipeline in case changes are detected, resulting in developers having to take action to fix the CI Pipeline.

It appears that the spec broke after https://gitlab.com/gitlab-org/gitlab/-/issues/37322, when the projects import/export feature shifted from a deny list to an allow list, which resulted in new attributes being included without developers evaluating whether they should be featured in the import_export.yml file.

For example, the project_ci_cd_settings table has a total of 21 settings/columns that might be exported or imported, but only 3 are migrated. It appears that these settings were added gradually. Many may have been missed when considering migration. However, it's not easy to verify this without a reliable source of truth documenting whether a setting was ignored for valid reasons.

Moreover, the specs currently only recognize changes made to projects, while it would be ideal to have similar specs to identify changes to groups.

Ideas

New attributes are easier to handle because, in most cases, developers only need to add the new attribute to the import_export.yml and do not need to make any other changes. Therefore, in most cases, they can self-serve.

When adding the new attribute, developers should consider the associated security risks. For example, they need to be mindful that importing users can manipulate such attributes before importing them. These considerations should be documented, and ultimately, developers could ping groupimport and integrate or AppSec for guidance.

For attributes considered unsafe to migrate, developers should document the reason, creating a single source of truth that makes it easy to find the justification. Similarly, if the attribute is not ready to be migrated, developers should create an issue and link it to the documentation.

Proposed solution

  • Update Project and Group migrations on Direct Transfer and Import/Export to use attribute permittee instead of attribute cleaner
  • Move excluded_attributes from the import_export.yml to a different YAML file, for example ignored_exported_attributes.yml. The reason for this is to make the import_export.yml tidy.
  • Develop an improved syntax for the ignored_exported_attributes.yml. The syntax should enable developers to provide a reason for why the attribute has been ignored or outline the issue that will initiate the migration of the attribute.

Example of YAML syntax:

Project:
  runners_token:
    note: "Has sensitive information"

ProjectCiCdSetting:
  forward_deployment_enabled:
    migrate_issue_url: "https://gitlab.com/gitlab-org/gitlab/-/issues/xxx"
  • Refactor the specs to detect new attributes. The spec should be well documented and inform developers that they can find more information on https://docs.gitlab.com/development/import_export/
  • Create a spec to detect new attributes in exported tables associated with groups.
  • Improve https://docs.gitlab.com/development/import_export/ documentation
  • As part of this issue, we will discover several fields that should be migrated but aren't. We should consider adding them to the import_export.yml, or adding them to the ignore list and creating issues to migrate them.
  • Add groupimport and integrate as a CODE OWNER of the ignored_exported_attributes.yml, so whenever someone updates the file, a member of our team would need to approve the MR, and we would get someone notified.
Edited by 🤖 GitLab Bot 🤖