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

Add RBAC permissions so Linseed can operate on secrets and configmaps #2883

Merged

Conversation

Josh-Tigera
Copy link
Contributor

where necessary

Description

For PR author

  • Tests for change.
    - [ ] If changing pkg/apis/, run make gen-files
    - [ ] If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

{
APIGroups: []string{""},
Resources: []string{"secrets"},
Verbs: []string{"get", "list", "watch"},
Copy link
Member

Choose a reason for hiding this comment

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

I think we can include a ResourceNames section here to limit specifically to the secret we want to copy, no? It should always be a well-known secret name + namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me check this and get back to you. I'm concerned about how well the shared informer's "list" and "watch" will handle an explicit list of ResourceNames.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's OK to do that in parallel and we can merge this to at least get things working.

It should definitely be possible to have the informer only watch a particular resource, it might require multiple in order to watch multiple resources though.

@caseydavenport
Copy link
Member

@Josh-Tigera looks like two UTs failing but otherwise I think we can merge

@marvin-tigera
Copy link
Contributor

Removing "merge-when-ready" label due to new commits

@marvin-tigera
Copy link
Contributor

Removing "merge-when-ready" label due to new commits

@marvin-tigera marvin-tigera merged commit 4c86b90 into tigera:master Sep 26, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants