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

Scorecard optional Config.yaml Service Account selection #5245

Merged

Conversation

theishshah
Copy link
Member

This enables selecting the service account for the scorecard using the config.yaml in addition to the existing method of changing it at runtime in the CLI.

This blocked on an API release which should occur shortly.

Ish Shah added 2 commits September 28, 2021 08:32
Signed-off-by: Ish Shah <[email protected]>
Signed-off-by: Ish Shah <[email protected]>
@theishshah theishshah changed the title [BLOCKED] Scorecard optional Config.yaml Service Account selection Scorecard optional Config.yaml Service Account selection Sep 28, 2021
@theishshah
Copy link
Member Author

v0.10.6 has been released in operator-framework/api allowing this PR to proceed.

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

Simplify the logic a bit more.

Comment on lines 201 to 206
runnerSA := ""
if o.Config.ServiceAccount != "" {
runnerSA = o.Config.ServiceAccount
} else {
runnerSA = c.serviceAccount
}
Copy link
Member

Choose a reason for hiding this comment

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

Since you're already initializing it, might as well set it to the default value

Suggested change
runnerSA := ""
if o.Config.ServiceAccount != "" {
runnerSA = o.Config.ServiceAccount
} else {
runnerSA = c.serviceAccount
}
runnerSA := c.serviceAccount
if o.Config.ServiceAccount != "" {
runnerSA = o.Config.ServiceAccount
}

Ish Shah added 2 commits September 28, 2021 08:54
Signed-off-by: Ish Shah <[email protected]>
Signed-off-by: Ish Shah <[email protected]>
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2021
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@theishshah theishshah merged commit bc8aedd into operator-framework:master Sep 28, 2021
@theishshah theishshah deleted the scConfigServiceaccount branch September 28, 2021 16:39
@theishshah theishshah restored the scConfigServiceaccount branch September 28, 2021 16:39
@asmacdo asmacdo added this to the v1.13.0 milestone Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants