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

feat(core): add namespace provider #5249

Closed
wants to merge 11 commits into from
Closed

feat(core): add namespace provider #5249

wants to merge 11 commits into from

Conversation

zachsmith1
Copy link
Contributor

@zachsmith1 zachsmith1 commented Feb 12, 2021

The new NamespaceProvider will support querying accounts for namespaces, spaces, etc. There are places across spinnaker, like Deck, that need this information to populate fields for users. Before this change, this data is gathered by calling the /credentials endpoint and then parsing the namespaces/etc from that account. It doesn't make sense to continue to parse this information from a credentials object directly. Allowing this data to be fetched independently allows for better scalability and control.

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@zachsmith1 zachsmith1 marked this pull request as draft February 12, 2021 20:15
@zachsmith1 zachsmith1 marked this pull request as ready for review March 1, 2021 19:46
import com.netflix.spinnaker.clouddriver.documentation.Empty;
import java.util.Collection;

public interface NamespaceProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using the term region in many places (coming from AWS), location in others. I'm hesitant to add one more term. I think it only matches Kubernetes (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Namespace in its literal sense is a k8 object but the abstraction is more or less what I was trying to accomplish. Namespace being a logical grouping around some resource. This happens to be a 1-1 with k8 and very close to 1-1 with CF (Org + Space combo). These are also dynamic in nature and can be created/updated/deleted. The tight coupling between the credentials and these "namespaces" is the main reason for this PR. Regions being more static might not make sense in this light

I'm open to calling it something different if you have a better abstraction in mind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking location to be consistent. It's not a very strongly held feeling on my side, I just wanted you to be aware that it's yet another naming scheme. Alternative idea: we could define: /credentials/kubernetes/[account name]/namespaces for Kubernetes, /credentials/cloudfoundry/[account name]/spaces for CF, etc. I guess it depends how you plan to use this in Deck.

@zachsmith1 zachsmith1 closed this Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants