✨ Add PullSecret controller to save pull secret data locally#1322
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1322 +/- ##
==========================================
- Coverage 76.42% 74.99% -1.44%
==========================================
Files 41 42 +1
Lines 2431 2507 +76
==========================================
+ Hits 1858 1880 +22
- Misses 400 443 +43
- Partials 173 184 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d2dea55 to
500da77
Compare
500da77 to
7e1b8f7
Compare
|
Where the PR stands so far:
More details about this file here
|
|
fixes #1015 |
7e1b8f7 to
bb1966c
Compare
bb1966c to
b5a5d25
Compare
| flag.StringVar(&cachePath, "cache-path", "/var/cache", "The local directory path used for filesystem based caching") | ||
| flag.BoolVar(&operatorControllerVersion, "version", false, "Prints operator-controller version information") | ||
| flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.") | ||
| flag.StringVar(&globalPullSecret, "global-pull-secret", "", "The namespace/name of the global pull secret that is going to be used to pull bundle images.") |
There was a problem hiding this comment.
Just as a thing to note, there is a flag.Var() method that would allow us to implement a custom flag value with custom logic for handling how it is set, including validations.
I did something similar to this in kapp back when we were contributing preflight checks: https://github.com/carvel-dev/kapp/blob/ca6887d26d782636fe62a2b3f3bbbf5c91a965f3/pkg/kapp/preflight/registry.go#L64-L97
Might be overkill for this particular case, but this may be a good way to handle any custom flag validations we want in the future
aeadb18 to
d553087
Compare
60a4c92 to
ec18c90
Compare
ec18c90 to
6cdef28
Compare
6cdef28 to
3741d4a
Compare
3741d4a to
bac538c
Compare
| cacheOptions.ByObject[&corev1.Secret{}] = crcache.ByObject{ | ||
| Namespaces: map[string]crcache.Config{ | ||
| globalPullSecretKey.Namespace: { | ||
| LabelSelector: k8slabels.Everything(), | ||
| FieldSelector: fields.SelectorFromSet(map[string]string{ | ||
| "metadata.name": globalPullSecretKey.Name, | ||
| }), | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Does this also override the configuration for secrets in the systemNamespace? IIUC we want to make sure we have access to all the release secrets that get created by the secret driver we use for partitioning helm release secrets for larger bundles.
There was a problem hiding this comment.
Approved pending an answer to this
There was a problem hiding this comment.
Dirty secret: iirc, the release secret driver is still a live client. So checking whether that is working won't actually tell you anything.
There was a problem hiding this comment.
Also I don't actually see any configuration for secretes in the systemNamespace. There's configuration for &ocv1alpha1.ClusterExtension{} and &catalogd.ClusterCatalog{}, but nothing for Secrets.
Dismissing because I had a question I forgot about that should be verified
| } | ||
|
|
||
| setupLog.Info("set up manager") | ||
| cacheOptions := crcache.Options{ |
There was a problem hiding this comment.
Nitpick: If you can add some comments around what the below code doing it will add a lot to the readability of the code.
There was a problem hiding this comment.
I'm not convinced any further documentation is necessary. The crcache.Options struct is pretty well documented. Once the reader understands what the controller-runtime cache does and what Options can be used to configure, the code reads "configuring caching options for the objects we own + additional object Secret in a specific namespace".
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
bac538c to
018510e
Compare
* ✨ Add PullSecret controller to save pull secret data locally RFC: https://docs.google.com/document/d/1BXD6kj5zXHcGiqvJOikU2xs8kV26TPnzEKp6n7TKD4M/edit#heading=h.x3tfh25grvnv * main.go: improved cache configuration for watching pull secret Signed-off-by: Joe Lanford <joe.lanford@gmail.com> --------- Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
RFC: https://docs.google.com/document/d/1BXD6kj5zXHcGiqvJOikU2xs8kV26TPnzEKp6n7TKD4M/edit#heading=h.x3tfh25grvnv
Description
Reviewer Checklist