Skip to content

Conversation

@jsoriano
Copy link
Member

@jsoriano jsoriano commented Nov 7, 2023

No description provided.

@jsoriano jsoriano requested a review from a team November 7, 2023 10:29
@jsoriano jsoriano self-assigned this Nov 7, 2023
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @jsoriano

if err != nil {
log.Fatal(err)
}
rsaPublicKeyArmor, err := rsaKey.GetArmoredPublicKey()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not familiar with this library,
should this be using rsaKeyArmor instead of rsaKey?

Suggested change
rsaPublicKeyArmor, err := rsaKey.GetArmoredPublicKey()
rsaPublicKeyArmor, err := rsaKeyArmor.GetArmoredPublicKey()

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. rsaKey contains information about both the private key and its public key. "Armoring" here only refers to getting each one of the keys in PEM format, the format that elastic-package expects to find.

Copy link
Member Author

Choose a reason for hiding this comment

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

rsaKeyArmor is a string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah Ok!
Thanks!

Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

🚀

@jsoriano jsoriano merged commit 5c4acbb into elastic:main Nov 7, 2023
@jsoriano jsoriano deleted the test-signing-key branch November 7, 2023 11:56
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.

3 participants