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

cmd/build: don't expect key if alg is non-empty #5343

Conversation

srenatus
Copy link
Contributor

@srenatus srenatus commented Nov 2, 2022

Since alg has a default setting, we've gotten that error too often.

Follow-up to #5297 and #4972.

@srenatus srenatus force-pushed the sr/cmd/fix-signing-key-missing-error branch from 2f60ed9 to 631210b Compare November 2, 2022 12:33
@@ -350,8 +348,8 @@ func buildVerificationConfig(pubKey, pubKeyID, alg, scope string, excludeFiles [
}

func buildSigningConfig(key, alg, claimsFile, plugin string) (*bundle.SigningConfig, error) {
if key == "" && (plugin != "" || claimsFile != "" || alg != "") {
return nil, fmt.Errorf("specify the secret (HMAC) or path of the PEM file containing the private key (RSA and ECDSA)")
if key == "" && (plugin != "" || claimsFile != "") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

☝️ This line is the relevant change, the rest is noise and cleanup 🧹

@srenatus srenatus marked this pull request as ready for review November 2, 2022 12:40
philipaconrad
philipaconrad previously approved these changes Nov 2, 2022
Copy link
Contributor

@philipaconrad philipaconrad left a comment

Choose a reason for hiding this comment

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

LGTM! Just minor nits:

cmd/build.go Outdated Show resolved Hide resolved
Since `alg` has a default setting, we've gotten that error too often.

Follow-up to open-policy-agent#5297 and open-policy-agent#4972.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus
Copy link
Contributor Author

srenatus commented Nov 2, 2022

will merge when green

@srenatus srenatus merged commit 549cf49 into open-policy-agent:main Nov 2, 2022
@srenatus srenatus deleted the sr/cmd/fix-signing-key-missing-error branch November 2, 2022 12:53
@srenatus
Copy link
Contributor Author

srenatus commented Nov 2, 2022

I lied 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants