Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 PullSecret controller to save pull secret data locally #1322
✨ Add PullSecret controller to save pull secret data locally #1322
Changes from all commits
eac1e69
018510e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 whatOptions
can be used to configure, the code reads "configuring caching options for the objects we own + additional object Secret in a specific 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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending an answer to this
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.