Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
matheuscscp opened this issue Aug 6, 2024 · 2 comments
Labels
area/api API related issues and pull requests enhancement New feature or request

Comments

@matheuscscp
Copy link
Collaborator

See #1552 (comment)

@matheuscscp matheuscscp added enhancement New feature or request area/bucket Bucket related issues and pull requests area/api API related issues and pull requests labels Aug 6, 2024
@darkowlzz
Copy link
Contributor

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.

@matheuscscp matheuscscp changed the title Rename constants to have common part as a prefix and variations at the end for Bucket v1 API Rename constants being promoted to API v1 to have the common part as a prefix and variations at the end Aug 7, 2024
@matheuscscp matheuscscp removed the area/bucket Bucket related issues and pull requests label Aug 7, 2024
@darkowlzz
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API related issues and pull requests enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants