Skip to content

Fix blank values handling in the dependency proxy

David Fernandez requested to merge 436375-refine-model-validations into master

📞 Context

In Maven dependency proxy (&3610 - closed), we're adding the first iteration of the dependency proxy for packages, starting with the maven package format.

This feature requires user to configure an url and optional credentials. These have been encapsulated into a dependency proxy for packages settings object.

Specifically for the maven package format, users can set:

  • An url. Required.
  • A username. Optional.
  • A password. Optional.

We have some model validations in place to make sure that the only cases we accept are:

  1. The username and password are both nil.
  2. The username and password are both set.

When those columns were added (!120894 (merged)), we added this validation at two levels:

  1. The model class.
  2. The database (check constraint).

The problem is that both validations treat differently blank values, such as the empty string ''. For the model validation, an empty string is a blank value. Using the <column_name>? function will return true (as if no value has been set). However, on the database side, NULL or "" or two very different things. Given that the database validation relies on counting the amount of NULLs, we can have situations where the model validations pass but not the database ones.

This has been described in Maven dependency proxy: refine model validations (#436375 - closed).

🤔 What does this MR do and why?

I think there are multiple ways to solve this on the model class. Given that those are credentials values, we don't really expect blank values. As such, we can consider them as NULL values.

Based on the observation above, this MR:

  • adds a before_validation callback that will set the value to nil for blank values in credentials columns.
  • updates the related specs.

The dependency proxy for Maven is still behind a feature flag. Hence, no changelog.

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.

🖼 Screenshots or screen recordings

Before After
Screenshot_2023-12-27_at_14.59.40 Screenshot_2023-12-27_at_14.58.59

🔧 How to set up and validate locally

The maven dependency for packages has a few requirements:

  1. have packages -> enabled set to true in gitlab.yml.

  2. have dependency_proxy -> enabled set to true in gitlab.yml.

  3. have the packages feature enabled in the project's settings. Settings -> General -> Visiblity, project features, permissions -> Package registry (checkbox enabled.)

  4. have a GitLab license. Premium or more.

  5. have the related feature flag turned on:

    Feature.enable(:packages_dependency_proxy_maven)

Now, things are ready. Have a project ready.

  1. Go to the graphql editor: http://gdk.test:8000/-/graphql-explorer

  2. Here is the mutation that will create or update the dependency proxy settings object:

    mutation {
      updateDependencyProxyPackagesSettings(input: {
        projectPath: "<project path>",
        mavenExternalRegistryPassword: "",
        mavenExternalRegistryUrl: "https://repo1.maven.org/maven3",
        mavenExternalRegistryUsername: null
      }) {
        dependencyProxyPackagesSetting {
          mavenExternalRegistryUrl
          mavenExternalRegistryUsername
        },
        errors
      }
    }
    • Notice that the GraphQL editor will have some warnings due to using a deprecated mutation. That's correct because that mutation is still behind a feature flag.
    • Note how the password is set to "" but the username is set to null. Previously, this would 💥 but with this MR, the "" is transformed into NULL for the database = both username + password are NULL, that's a valid case
Edited by David Fernandez

Merge request reports