-
Notifications
You must be signed in to change notification settings - Fork 702
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
No more secret requests to K8s API #4034
Conversation
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>
if (repo?.spec?.dockerRegistrySecrets?.some(s => s === secretName)) { | ||
setSelectedImagePullSecret(secretName); | ||
} | ||
}); | ||
}, [imagePullSecrets, repo, selectedImagePullSecret]); | ||
}, [imagePullSecrets, repo]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed selectedImagePullSecret
from the dependencies here, as it was causing a bug: every time you selected a different image pull secret, it would trigger this effect, which would set the selected image pull secret back to the one that matches the repos current reference, effectively stopping you from ever changing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! From now on, the browser's console will look way better without all these secret requests!
}); | ||
|
||
const secretNames = []; | ||
for (const [name, type] of Object.entries(result.secretNames)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps less wordy with a for..in statement but fine either way
// TODO(andresmgot): Create an endpoint for returning just the list of secret names | ||
// to avoid listing all the secrets with protected information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are finally doing it :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was sad I couldn't close the related issue (because it was already closed, not sure why).
Description of the change
This PR follows on from #4028, updating the AppRepoForm so that it fetches the image pull secret names only (from the resources API), rather than fetching (and storing in global state) the image pull secrets for a namespace. Also updated, while there, to no longer use global state for these, but rather just local state on the form.
It also does some cleaning up, so that
shared/Secret.ts
no longer has any references to the k8s API, as well as removing the urls fromshared/urls
etc.Tested IRL.
Benefits
Kubeapps no longer hits the k8s API for secrets, nor does it expose any method for fetching secrets generally.
Possible drawbacks
Applicable issues
Additional information