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

Add implementation for resources secret endpoints #3915

Merged
merged 5 commits into from
Jan 4, 2022

Conversation

absoludity
Copy link
Contributor

Description of the change

Following on from #3914, this PR adds the implementations for CreateSecret and GetSecretNames.

Benefits

Can now update the dashboard to use the new endpoints rather than hitting the k8s API server.

Possible drawbacks

Applicable issues

Additional information

@castelblanque
Copy link
Collaborator

Great, now Kubeapps is not exposing secret contents anymore, and we are moving away from k8s API server calls.
Thanks!

@absoludity
Copy link
Contributor Author

Great, now Kubeapps is not exposing secret contents anymore,

Not for the dashboard endpoints that will switch to use this API. Note that the GetResourcesForInstalledPackage still returns the secrets for an installed package just like any other resource (and they're used in the dashboard), but we can eventually remove that too so that we only fetch the actual secret value if the user chooses to view one. But that's for another day.

and we are moving away from k8s API server calls. Thanks!

Yay to that :)

Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Great! Another step forward!

Comment on lines 40 to 42
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
Kind: "Secret",
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to avoid harcoding these literals, I opted for adding the corev1 k8s API as a dependency and just did:

		TypeMeta: metav1.TypeMeta{
			Kind:       k8scorev1.ResourceSecrets.String(),
			APIVersion: k8scorev1.SchemeGroupVersion.WithResource(k8scorev1.ResourceSecrets.String()).String(),
		},
...

See what you think, but anyway, I'll try to be as consistent as possible to avoid future issues when upgrading k8s versions.
I mean, I'm happy to change the kapp-related code I wrote to use the literals directly if you think otherwise, but, you know, we better keep it uniform.

Note: this comment also applies to the CreateNamespace function, but it was already merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll update to that.

@stale
Copy link

stale bot commented Dec 29, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Automatic label to stale issues due inactivity to be closed if no further action label Dec 29, 2021
@stale stale bot closed this Jan 1, 2022
@antgamdia antgamdia reopened this Jan 3, 2022
@stale stale bot removed the stale Automatic label to stale issues due inactivity to be closed if no further action label Jan 3, 2022
plugin.

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity merged commit fdb9861 into master Jan 4, 2022
@absoludity absoludity deleted the 3896-secrets-backend-2 branch January 4, 2022 05:10
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.

None yet

3 participants