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

to plug service account informer to podsecuritypolicyreview #11612

Conversation

sdminonne
Copy link
Contributor

@Kargakis as discussed here
@soltysh fyi

@@ -97,7 +99,7 @@ func (r *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
return pspr, nil
}

func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, client clientset.Interface, namespace string) ([]*kapi.ServiceAccount, error) {
func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, saCache *oscache.StoreToServiceAccountLister, namespace string) ([]*kapi.ServiceAccount, error) {
serviceAccounts := []*kapi.ServiceAccount{}
// TODO: express 'all service accounts'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still want to do this? It seems to me that you want just the service accounts associated with the psp, right?

@0xmichalis
Copy link
Contributor

Looks good, had an unrelated question.

@sdminonne sdminonne force-pushed the sa_informer_in_posecuritypolicyreview branch from 0d677a1 to dc761fb Compare October 27, 2016 13:22
@sdminonne
Copy link
Contributor Author

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to dc761fb

@0xmichalis
Copy link
Contributor

@mfojtik this LGTM - tag when possible

@0xmichalis 0xmichalis added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2016
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10787/) (Base Commit: 4e99352)

@soltysh
Copy link
Contributor

soltysh commented Nov 1, 2016

@mfojtik mind merging this one?

@mfojtik
Copy link
Contributor

mfojtik commented Nov 2, 2016

[merge]

@soltysh
Copy link
Contributor

soltysh commented Nov 2, 2016

Flake #11661.

@mfojtik
Copy link
Contributor

mfojtik commented Nov 2, 2016

re [merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to dc761fb

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 2, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10787/) (Base Commit: 21040e2) (Image: devenv-rhel7_5300)

@openshift-bot openshift-bot merged commit 66834ff into openshift:master Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security lgtm Indicates that a PR is ready to be merged. priority/P2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants