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

Providers only allow one namespace, but Spinnaker allows many to be used. #128

Open
skandragon opened this issue Apr 7, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@skandragon
Copy link
Contributor

This will likely require a migration and splitting the single table apart, unless we do something gross like splitting a string.

@billiford billiford added the bug Something isn't working label Apr 7, 2022
@billiford
Copy link
Collaborator

I've given this some thought and I think we need to provide a backwards compatible approach for now.

We should add a namespaces field to the provider, so the provider JSON looks something like:

{
	"name": "provider-name",
	"host": "provider-host",
	"caData": "provider-ca-data",
	"tokenProvider": "provider-token-provider",
	"namespace": "OPTIONAL",
        "namespaces": [
               "OPTIONAL"
       ]
}

Some fields redacted for legibility. Having both the namespace and namespaces fields is confusing and we should deprecate the namespace field for a future release to remove it.

We should create a new SQL table kubernetes_providers_namespaces with an association clause. The table would have two columns for account_name and namespace, allowing a one-to-many association of accounts to namespaces.

  • All references to provider.Namespace throughout the code must be replaced with references to provider.Namespaces
  • All associated logic with provider.Namespace must be tested and modified to work with the namespaces array
  • When retrieving providers append any namespace stored in the kubernetes_providers table to the namespaces array
  • When creating/updating a provider append the namespace field (if set) to the namespaces array
  • Update the swagger def to note these changes, and if possible note that the namespace field is deprecated
  • Create an issue to remove the namespace field in a future release

@billiford billiford added enhancement New feature or request and removed bug Something isn't working labels Apr 18, 2022
@skandragon
Copy link
Contributor Author

Seems like a sane approach. I'd also make sure the API calls have at most one of namespace or namespaces set. What do the GET requests look like in this context, and PUT requests? If I set namespaces will that automatically clear namespace?

@billiford
Copy link
Collaborator

The idea now is just to allow setting both. If both are set, then the value of namespace is appended to the namespaces array on create/update.

Let me know if this answers your question.

@billiford
Copy link
Collaborator

@abe21412 @guido9j we can close this since #138 implements this enhancement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants