-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(git): enhance galaxyGitEnv for private repository authentication and add tests #4156
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
Open
befika
wants to merge
12
commits into
semaphoreui:develop
Choose a base branch
from
befika:sem-122-problem-support-for-private-git-collectionsroles
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+447
−2
Open
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
60c241e
feat(git): enhance galaxyGitEnv for private repository authentication…
befika 6278c35
feat(install): enhance InstallRequirements to support SSH key install…
befika adc5b81
Correct comment on single quotes in sqQuote
fiftin 4bb51ea
Merge branch 'develop' into sem-122-problem-support-for-private-git-c…
befika 15a0175
feat(AnsibleApp): enhance galaxy key installation handling and improv…
befika e09b6a4
Merge branch 'sem-122-problem-support-for-private-git-collectionsrole…
befika a4bf9eb
feat: improve AnsibleApp functionality and enhance test coverage for …
befika 5bd7d1d
Update db_lib/GalaxyGitEnv_test.go
befika a5426ab
Merge branch 'sem-122-problem-support-for-private-git-collectionsrole…
befika 133bf0c
fix: update stubGalaxy to correctly log run output with quoted path
befika 0c31fd2
fix(galaxy): escape "=" in git credentials for GIT_CONFIG_PARAMETERS
befika 9ae4693
Merge branch 'develop' into sem-122-problem-support-for-private-git-c…
fiftin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,242 @@ | ||
| package db_lib | ||
|
|
||
| import ( | ||
| "errors" | ||
| "os" | ||
| "path" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/semaphoreui/semaphore/db" | ||
| "github.com/semaphoreui/semaphore/pkg/ssh" | ||
| "github.com/semaphoreui/semaphore/pkg/task_logger" | ||
| "github.com/semaphoreui/semaphore/util" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func httpRepo(gitURL, login, password string) db.Repository { | ||
| return db.Repository{ | ||
| GitURL: gitURL, | ||
| SSHKey: db.AccessKey{ | ||
| Type: db.AccessKeyLoginPassword, | ||
| LoginPassword: db.LoginPassword{Login: login, Password: password}, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func TestSqQuote(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| input string | ||
| expected string | ||
| }{ | ||
| {"plain", "abc", "'abc'"}, | ||
| {"empty", "", "''"}, | ||
| {"embedded quote", "a'b", `'a'\''b'`}, | ||
| {"only quote", "'", `''\'''`}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| assert.Equal(t, tt.expected, sqQuote(tt.input)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestGalaxyGitEnv_AlwaysDisablesTerminalPrompt(t *testing.T) { | ||
| for _, repo := range []db.Repository{ | ||
| {GitURL: "git@github.com:acme/roles.git"}, | ||
| {GitURL: "https://git.private.repo/acme/roles.git"}, | ||
| httpRepo("https://git.private.repo/acme/roles.git", "u", "p"), | ||
| } { | ||
| assert.Contains(t, galaxyGitEnv(repo), "GIT_TERMINAL_PROMPT=0") | ||
| } | ||
| } | ||
|
|
||
| func TestGalaxyGitEnv_HTTPSWithLoginPassword(t *testing.T) { | ||
| env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "semuser", "sempass")) | ||
|
|
||
| require.Len(t, env, 2) | ||
| assert.Equal(t, | ||
| `GIT_CONFIG_PARAMETERS='url.https://semuser:sempass@git.private.repo/.insteadOf=https://git.private.repo/'`, | ||
| env[1]) | ||
| } | ||
|
|
||
| // The rewrite must match host and port, or git will not apply it. | ||
| func TestGalaxyGitEnv_KeepsPort(t *testing.T) { | ||
| env := galaxyGitEnv(httpRepo("http://127.0.0.1:3300/semuser/main-repo.git", "semuser", "sempass")) | ||
|
|
||
| require.Len(t, env, 2) | ||
| assert.Contains(t, env[1], "url.http://semuser:sempass@127.0.0.1:3300/.insteadOf=http://127.0.0.1:3300/") | ||
| } | ||
|
|
||
| // Matches GetGitURL: an empty login means the password is the whole credential. | ||
| func TestGalaxyGitEnv_TokenOnlyKeyUsesPasswordAsUser(t *testing.T) { | ||
| env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "", "gho_token")) | ||
|
|
||
| require.Len(t, env, 2) | ||
| assert.Contains(t, env[1], "url.https://gho_token@git.private.repo/.insteadOf=") | ||
| } | ||
|
|
||
| // A password containing '@' or '/' would otherwise corrupt the URL. | ||
| func TestGalaxyGitEnv_EncodesCredentials(t *testing.T) { | ||
| env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "user@corp", "p@ss/w:rd")) | ||
|
|
||
| require.Len(t, env, 2) | ||
| assert.Contains(t, env[1], "user%40corp:p%40ss%2Fw%3Ard@git.private.repo") | ||
| assert.NotContains(t, env[1], "p@ss/w:rd") | ||
| } | ||
|
|
||
| func TestGalaxyGitEnv_NoCredentialsForOtherRepoTypes(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| repo db.Repository | ||
| }{ | ||
| {"ssh url", db.Repository{ | ||
| GitURL: "git@github.com:acme/roles.git", | ||
| SSHKey: db.AccessKey{Type: db.AccessKeyLoginPassword, | ||
| LoginPassword: db.LoginPassword{Login: "u", Password: "p"}}, | ||
| }}, | ||
| {"https url but ssh key", db.Repository{ | ||
| GitURL: "https://git.private.repo/acme/roles.git", | ||
| SSHKey: db.AccessKey{Type: db.AccessKeySSH}, | ||
| }}, | ||
| {"https url but no key", db.Repository{ | ||
| GitURL: "https://git.private.repo/acme/roles.git", | ||
| SSHKey: db.AccessKey{Type: db.AccessKeyNone}, | ||
| }}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| env := galaxyGitEnv(tt.repo) | ||
| assert.Equal(t, []string{"GIT_TERMINAL_PROMPT=0"}, env) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // requirements.yml may name several servers; only the repository's own host | ||
| // may ever be offered its credential. | ||
| func TestGalaxyGitEnv_ScopesCredentialToOneHost(t *testing.T) { | ||
| env := galaxyGitEnv(httpRepo("https://git.private.repo/acme/roles.git", "semuser", "sempass")) | ||
|
|
||
| require.Len(t, env, 2) | ||
| assert.Contains(t, env[1], ".insteadOf=https://git.private.repo/") | ||
| assert.NotContains(t, env[1], "acme/roles.git") | ||
| } | ||
|
|
||
| type fakeInstaller struct { | ||
| key db.AccessKey | ||
| usage db.AccessKeyRole | ||
| env []string | ||
| err error | ||
| calls int | ||
| } | ||
|
|
||
| func (f *fakeInstaller) Install(key db.AccessKey, usage db.AccessKeyRole, _ task_logger.Logger) (ssh.AccessKeyInstallation, error) { | ||
| f.key, f.usage = key, usage | ||
| f.calls++ | ||
| return ssh.AccessKeyInstallation{}, f.err | ||
| } | ||
|
|
||
| // setupGalaxyConfig gives the package-level util.Config a temp dir to resolve | ||
| // repository paths against, and restores it afterwards. | ||
| func setupGalaxyConfig(t *testing.T) { | ||
| original := util.Config | ||
| t.Cleanup(func() { util.Config = original }) | ||
| util.Config = &util.ConfigType{TmpPath: t.TempDir(), Process: &util.ConfigProcess{}} | ||
| } | ||
|
|
||
| // stubGalaxy puts a succeeding ansible-galaxy first on PATH and returns a | ||
| // function reporting how many times it ran. Without it the real binary runs and | ||
| // fails, so InstallRequirements returns before the second requirements file and | ||
| // nothing is reused. | ||
| func stubGalaxy(t *testing.T) func() int { | ||
| t.Helper() | ||
|
|
||
| dir := t.TempDir() | ||
| runLog := path.Join(dir, "runs") | ||
| script := "#!/bin/sh\necho run >> " + runLog + "\n" | ||
|
befika marked this conversation as resolved.
Outdated
|
||
| require.NoError(t, os.WriteFile(path.Join(dir, "ansible-galaxy"), []byte(script), 0o755)) | ||
| t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) | ||
|
|
||
| return func() int { | ||
| content, err := os.ReadFile(runLog) | ||
| if err != nil { | ||
| return 0 | ||
| } | ||
| return strings.Count(string(content), "run") | ||
| } | ||
| } | ||
|
|
||
| // newGalaxyApp builds the app the way AppFactory does, so that runGalaxy has a | ||
| // Playbook to run. | ||
| func newGalaxyApp(repo db.Repository) *AnsibleApp { | ||
| logger := task_logger.NopLogger{} | ||
|
|
||
| return &AnsibleApp{ | ||
| Logger: logger, | ||
| Repository: repo, | ||
| Playbook: &AnsiblePlaybook{Repository: repo, Logger: logger}, | ||
| } | ||
| } | ||
|
|
||
| // writeRequirements puts a requirements.yml where the app looks for it, so that | ||
| // galaxy actually runs. Without one every install is skipped. | ||
| func writeRequirements(t *testing.T, app *AnsibleApp) { | ||
| t.Helper() | ||
|
|
||
| dir := app.getRepoPath() | ||
| require.NoError(t, os.MkdirAll(dir, 0o755)) | ||
| require.NoError(t, os.WriteFile(path.Join(dir, "requirements.yml"), []byte("collections: []\n"), 0o644)) | ||
| } | ||
|
|
||
| // The repository's own key must be the one galaxy gets, under the git role. | ||
| func TestInstallRequirements_InstallsRepositoryKey(t *testing.T) { | ||
| setupGalaxyConfig(t) | ||
|
|
||
| inst := &fakeInstaller{} | ||
| app := newGalaxyApp(db.Repository{SSHKey: db.AccessKey{ID: 42, Type: db.AccessKeySSH}}) | ||
| writeRequirements(t, app) | ||
| galaxyRuns := stubGalaxy(t) | ||
|
|
||
| require.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{Installer: inst})) | ||
|
|
||
| require.Greater(t, galaxyRuns(), 1, "reuse is only meaningful across more than one galaxy run") | ||
| assert.Equal(t, 42, inst.key.ID) | ||
| assert.Equal(t, db.AccessKeyRole(db.AccessKeyRoleGit), inst.usage) | ||
| assert.Equal(t, 1, inst.calls, "one installation must be reused across requirements files") | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // Nothing for galaxy to install means no key is decrypted and no agent started. | ||
| func TestInstallRequirements_NoRequirementsFileInstallsNoKey(t *testing.T) { | ||
| setupGalaxyConfig(t) | ||
|
|
||
| inst := &fakeInstaller{} | ||
| app := newGalaxyApp(db.Repository{SSHKey: db.AccessKey{ID: 42, Type: db.AccessKeySSH}}) | ||
|
|
||
| require.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{Installer: inst})) | ||
|
|
||
| assert.Zero(t, inst.calls) | ||
| } | ||
|
|
||
| func TestInstallRequirements_FailsWhenKeyInstallFails(t *testing.T) { | ||
| setupGalaxyConfig(t) | ||
|
|
||
| app := newGalaxyApp(db.Repository{}) | ||
| writeRequirements(t, app) | ||
|
|
||
| err := app.InstallRequirements(LocalAppInstallingArgs{ | ||
| Installer: &fakeInstaller{err: errors.New("agent unavailable")}, | ||
| }) | ||
|
|
||
| assert.ErrorContains(t, err, "agent unavailable") | ||
| } | ||
|
|
||
| // A nil installer is the remote-runner path; it must not panic. | ||
| func TestInstallRequirements_NilInstaller(t *testing.T) { | ||
| setupGalaxyConfig(t) | ||
|
|
||
| app := newGalaxyApp(db.Repository{}) | ||
|
|
||
| assert.NoError(t, app.InstallRequirements(LocalAppInstallingArgs{})) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.