-
Notifications
You must be signed in to change notification settings - Fork 43
feat: add env var for ssh private key #396
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
Changes from 3 commits
1cc9255
ff33361
4d3bb0e
d6e3ae0
01bf936
22d9230
c916826
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package git_test | |
import ( | ||
"context" | ||
"crypto/ed25519" | ||
"encoding/base64" | ||
"fmt" | ||
"io" | ||
"net/http/httptest" | ||
|
@@ -433,6 +434,22 @@ func TestSetupRepoAuth(t *testing.T) { | |
require.Equal(t, actualSigner, pk.Signer) | ||
}) | ||
|
||
t.Run("SSH/Base64PrivateKey", func(t *testing.T) { | ||
opts := &options.Options{ | ||
GitURL: "ssh://[email protected]:repo/path", | ||
GitSSHPrivateKeyBase64: base64EncodeTestPrivateKey(), | ||
} | ||
auth := git.SetupRepoAuth(t.Logf, opts) | ||
|
||
pk, ok := auth.(*gitssh.PublicKeys) | ||
require.True(t, ok) | ||
require.NotNil(t, pk.Signer) | ||
|
||
actualSigner, err := gossh.ParsePrivateKey([]byte(testKey)) | ||
require.NoError(t, err) | ||
require.Equal(t, actualSigner, pk.Signer) | ||
}) | ||
|
||
t.Run("SSH/NoAuthMethods", func(t *testing.T) { | ||
opts := &options.Options{ | ||
GitURL: "ssh://[email protected]:repo/path", | ||
|
@@ -502,3 +519,7 @@ func writeTestPrivateKey(t *testing.T) string { | |
require.NoError(t, os.WriteFile(kPath, []byte(testKey), 0o600)) | ||
return kPath | ||
} | ||
|
||
func base64EncodeTestPrivateKey() string { | ||
return base64.StdEncoding.EncodeToString([]byte(testKey)) | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -108,6 +108,9 @@ type Options struct { | |||||
// GitSSHPrivateKeyPath is the path to an SSH private key to be used for | ||||||
// Git authentication. | ||||||
GitSSHPrivateKeyPath string | ||||||
// GitSSHPrivateKeyBase64 is the content of an SSH private key to be used | ||||||
// for Git authentication. | ||||||
GitSSHPrivateKeyBase64 string | ||||||
// GitHTTPProxyURL is the URL for the HTTP proxy. This is optional. | ||||||
GitHTTPProxyURL string | ||||||
// WorkspaceFolder is the path to the workspace folder that will be built. | ||||||
|
@@ -363,6 +366,12 @@ func (o *Options) CLI() serpent.OptionSet { | |||||
Value: serpent.StringOf(&o.GitSSHPrivateKeyPath), | ||||||
Description: "Path to an SSH private key to be used for Git authentication.", | ||||||
}, | ||||||
{ | ||||||
Flag: "git-ssh-private-key-base64", | ||||||
Env: WithEnvPrefix("GIT_SSH_PRIVATE_KEY_BASE64"), | ||||||
Value: serpent.StringOf(&o.GitSSHPrivateKeyBase64), | ||||||
Description: "SSH private key to be used for Git authentication.", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The "This option takes ..." is just there for completeness, may be rewritten depending on how the final implementation turns out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we error on having both provided, do we document this in both of the descriptions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, we should be explicit that both options are mutually exclusive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have documented this now 👍 |
||||||
}, | ||||||
{ | ||||||
Flag: "git-http-proxy-url", | ||||||
Env: WithEnvPrefix("GIT_HTTP_PROXY_URL"), | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth thinking about what the behavior should be if both key path and key base64 is given. Do we exit immediately (invalid input)? Do we prefer one over the other? How do we inform the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I think we should error if both are provided. Removes any ambiguity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with erroring, have made that change 👍