fluxcd/source-controller

Rename constants being promoted to API `v1` to have the common part as a prefix and variations at the end

matheuscscp opened this issue · 2 comments

darkowlzz commented

I thought more about it and looked for what we already have. I think this issue can be generalized and not be specific to Bucket API as we have the same naming issue for other APIs. Here are some more information that may be relevant and useful here.

I'll focus on third-party "provider" related constants only here as we have some space to fix them going into v1 API. The other constants related to conditions and reasons are ignored as they already exist in the v1 API package.

At present, in the v1 API package we have GitRepository, HelmRepository and HelmChart. Of these, only HelmRepository and HelmChart have some third party provider for performing certain operations. We will soon have contextual login in GitRepository, then it will be relevant too. HelmRepository, when used for OCI registries, have a provider field for authentication with the cloud providers. It accepts the usual values aws, azure, gcp and generic. Since these are related to OCI and OCIRepository is still in v1beta2, the constants for these values in v1beta2 API exist as AmazonOCIProvider, AzureOCIProvider, GoogleOCIProvider and GenericOCIProvider alongside the OCIRepository type definition. The v1 API package doesn't have these constants which can be used with HelmRepository yet. Maybe for good? Maybe intentional? It provides an opportunity to drop the old constants in v1 API if we decide to do that.
For OCI HelmChart artifact verification, there are two verification providers: cosign and notation. For some reason, we never created constants for these. Maybe because it was just cosign until a few months ago and there was no need for it. Since there are no constants or variables, these can be ignored for now. But if we decide to introduce constants, we can name them with the proposed pattern, something like OCIVerificationProviderCosign and OCIVerificationProviderNotation.

In v1 API package, we can create new constants, for example:

const (
	OCIProviderGeneric string = "generic"
	OCIProviderAmazon  string = "aws"
	OCIProviderGoogle  string = "gcp"
	OCIProviderAzure   string = "azure"

	BucketProviderGeneric string = "generic"
	BucketProviderAmazon  string = "aws"
	BucketProviderGoogle  string = "gcp"
	BucketProviderAzure   string = "azure"
)

and if we decide to make it easy for users to use the v1 API, without needing to change the variables, create alias for the old constants with these new ones, for example:

var (
	GenericOCIProvider = OCIProviderGeneric
	AmazonOCIProvider  = OCIProviderAmazon
	GoogleOCIProvider  = OCIProviderGoogle
	AzureOCIProvider   = OCIProviderAzure

	GenericBucketProvider = BucketProviderGeneric
	AmazonBucketProvider  = BucketProviderAmazon
	GoogleBucketProvider  = BucketProviderGoogle
	AzureBucketProvider   = BucketProviderAzure
)

In the examples above, the providers are the same, we could have common constants for both OCI and Bucket, but this may just be a coincidence for these two APIs. The constants for the upcoming Git providers will have other values, "github", "gitlab", etc. Maintaining separate providers for separate APIs seems better.

  • 👍2
darkowlzz commented

This was discussed in the dev meeting today and it was decided to introduce the new constants in v1beta2 API package first, updating the existing constants to be an alias to the new constants and marking them as deprecated. The old constants will never be removed from v1beta2 API, they will only be marked as deprecated. In the v1 API package, only the new constants will be introduced. The underlying values for the constants will remain the same, no user facing change. Only the consumers of these constants will have to migrate to the new names.

  • 👍1