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 configuration on Agent to set X509SVID key type #3237

Merged
merged 3 commits into from
Jul 19, 2022

Conversation

MarcosDY
Copy link
Collaborator

Add configuration on Agent to set X509SVID key type

Which issue this PR fixes
fixes #3226

Signed-off-by: Marcos Yacob <marcos.yacob@hpe.com>
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

Thanks, @MarcosDY! Just a few comments.

msg: "workload_key_type invalid value",
expectError: true,
input: func(c *Config) {
c.Agent.WorkloadKeyType = "no a key"
Copy link
Member

Choose a reason for hiding this comment

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

nit: "not a key"

return nil, fmt.Errorf("unknown key type %q", keyType)
}

func (keyType KeyType) SignatureAlgorithm() (x509.SignatureAlgorithm, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we shouldn't just rely on the Go standard library choice for the signature algorithm....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done, I removed SignatureAlgorithm and rely on Go standard library

@@ -69,6 +70,7 @@ type agentConfig struct {
ServerAddress string `hcl:"server_address"`
ServerPort int `hcl:"server_port"`
SocketPath string `hcl:"socket_path"`
WorkloadKeyType string `hcl:"workload_key_type"`
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Wonder if we shouldn't be more specific that this is for the X509-SVID.... workload_x509_svid_key_type? What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like workload_x509_svid_key_type I was thinking on something like that, to avoid confusion

@azdagron azdagron self-assigned this Jul 19, 2022
Signed-off-by: Marcos Yacob <marcos.yacob@hpe.com>
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

lgtm!

@azdagron azdagron merged commit 9eb12df into spiffe:main Jul 19, 2022
@azdagron azdagron added this to the 1.4.0 milestone Jul 20, 2022
@MarcosDY MarcosDY deleted the allow-set-workload-key-type branch February 17, 2023 17:18
stevend-uber pushed a commit to stevend-uber/spire that referenced this pull request Oct 16, 2023
Signed-off-by: Marcos Yacob <marcos.yacob@hpe.com>
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.

Agent using different Workload key types
3 participants