-
Notifications
You must be signed in to change notification settings - Fork 432
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
bugfix: set labels to kubeconfig and cert secrets #2330
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for vcluster-docs canceled.Built without sensitive environment variables
|
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.
@maheshbaliga thanks for the PR! Please see my comments
pkg/util/kubeconfig/kubeconfig.go
Outdated
@@ -70,6 +73,10 @@ func WriteKubeConfig(ctx context.Context, currentNamespaceClient client.Client, | |||
ObjectMeta: metav1.ObjectMeta{ | |||
Name: secretName, | |||
Namespace: secretNamespace, | |||
Labels: map[string]string{ | |||
KubeConfigSecretLabelAppKey: KubeConfigSecretLabelAppValue, | |||
KubeConfigSecretLabelReleaseKey: secretNamespace, |
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 don't think the release name is equal to the namespace name, that only happens for development where the release is equal the namespace
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.
Hi @FabianKramm, I have made the necessary changes. I initially wanted to use 'vcluster.loft.sh/vcluster-name' instead of 'vcluster-name' as the key, but found the same key used as an annotation in syncer.go
pkg/certs/ensure.go
Outdated
@@ -131,6 +137,10 @@ func EnsureCerts( | |||
Name: secretName, | |||
Namespace: currentNamespace, | |||
OwnerReferences: ownerRef, | |||
Labels: map[string]string{ | |||
CertSecretLabelAppKey: CertSecretLabelAppValue, | |||
CertSecretLabelReleaseKey: currentNamespace, |
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 don't think the release name is equal to the namespace name, that only happens for development where the release is equal the namespace
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 have made the necessary changes.
bdf3abb
to
dbf9d38
Compare
What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>
if possible)resolves #2283 .The pull request adds labels to two secrets that contain a kubeconfig and a certificate. These labels help improve the organization and management of the secrets.
Please provide a short message that should be published in the vcluster release notes
Fixed an issue where vcluster 2 secrets containing a kubeconfig and a certificate were missing labels.
What else do we need to know?