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

Use correct secret in kubeapps namespace when syncing an app repo #1525

Merged
merged 3 commits into from
Feb 19, 2020

Conversation

absoludity
Copy link
Contributor

Updates the cronjob spec so that, when an app repository is from a namespace other than kubeapps, the correct secret ref is used for the repo.

Ref: #1521 .

@absoludity absoludity self-assigned this Feb 18, 2020
}
}

// jobSpec returns a batchv1.JobSpec for running the chart-repo sync job
func syncJobSpec(apprepo *apprepov1alpha1.AppRepository) batchv1.JobSpec {
func syncJobSpec(apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string) batchv1.JobSpec {
Copy link
Contributor

Choose a reason for hiding this comment

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

here (line 457) you also need to change the secretKeyRef?

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, thanks.

Comment on lines 587 to 589
if apprepo.ObjectMeta.Namespace != kubeappsNamespace {
secretKeyRef.LocalObjectReference.Name = kube.KubeappsSecretNameForRepo(apprepo.ObjectMeta.Name, apprepo.ObjectMeta.Namespace)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be simpler to just create a copy of the apprepo in the kubeapps namespace. That would help with the garbage collection and wouldn't require to change these secret references on the fly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would, if we weren't transitioning towards an end goal of syncing across all namespaces and running the jobs in those namespaces. This way we can begin syncing AppRepos across namespaces, we are just still running the sync jobs in the kubeapps namespace until we can update the assetsvc to receive data from the syncer.

Thanks!

@absoludity absoludity merged commit b772c3b into master Feb 19, 2020
@andresmgot andresmgot deleted the 1521-per-namespace-apps-synced-with-creds branch September 8, 2020 08:57
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.

2 participants