Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
- uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
with:
go-version-file: "go.mod"
- name: build
- name: unit-test
run: |
make test-unit-verbose-and-race
generated:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ spec:
# my-secret-token should be created in the namespace where the
# pipelinerun is created and contain a GitHub personal access
# token in the token key of the secret.
- name: token
# Can be created with the command:
# kubectl create secret generic my-secret-token --from-literal token=$RAW_TOKEN
- name: gitToken
value: my-secret-token
- name: tokenKey
- name: gitTokenKey
Comment on lines -34 to +38
Copy link
Member Author

Choose a reason for hiding this comment

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

The token and tokenKey fields are allowed here, however they're used for API-based authentication not http auth. So they're semantically incorrect to include with the url parameter. gitToken and gitTokenKey is correct.

Copy link
Member

Choose a reason for hiding this comment

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

sure but people may have used it previously, (if it ever got shipped in a working state)

value: token
params:
- name: url
Expand Down
27 changes: 13 additions & 14 deletions pkg/resolution/resolver/git/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (r remote) clone(ctx context.Context) (*repository, func(), error) {
os.RemoveAll(tmpDir)
}

repo := repository{
repo := &repository{
url: r.url,
username: r.username,
password: r.password,
Expand All @@ -62,7 +62,7 @@ func (r remote) clone(ctx context.Context) (*repository, func(), error) {
}
return nil, cleanupFunc, err
}
return &repo, cleanupFunc, nil
return repo, cleanupFunc, nil
}

type repository struct {
Expand Down Expand Up @@ -105,19 +105,18 @@ func (repo *repository) execGit(ctx context.Context, subCmd string, args ...stri
// We need to configure which directory contains the cloned repository since `cd`ing
// into the repository directory is not concurrency-safe
configArgs := []string{"-C", repo.directory}

env := []string{"GIT_TERMINAL_PROMPT=false"}
if subCmd == "clone" {
Copy link
Member Author

@aThorp96 aThorp96 Aug 2, 2025

Choose a reason for hiding this comment

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

This was removed as some git commands like fetch also require authentication. I believe this was originally here in the first place because a previous mechanism to supply authentication was specific to the git-clone subcommand, and that iteration git fetch was not being used. However --config-env is a globally accepted configuration and has no adverse effects on commands which don't require authentication.

// NOTE: Since this is only HTTP basic auth, authentication only supports http
// cloning, while unauthenticated cloning works for any other protocol supported
// by the git binary which doesn't require authentication.
if repo.username != "" && repo.password != "" {
token := base64.URLEncoding.EncodeToString([]byte(repo.username + ":" + repo.password))
env = append(
env,
"GIT_AUTH_HEADER=Authorization=Basic "+token,
)
configArgs = append(configArgs, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
}
// NOTE: Since this is only HTTP basic auth, authentication is only supported for http
// cloning, while unauthenticated cloning is supported for any other protocol supported
// by git which doesn't require authentication.
if repo.username != "" && repo.password != "" {
token := base64.URLEncoding.EncodeToString([]byte(repo.username + ":" + repo.password))
env = append(
env,
"GIT_AUTH_HEADER=Authorization: Basic "+token,
Copy link
Member Author

Choose a reason for hiding this comment

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

Authorization: Basic .... vs Authorization=Basic ... appears to have been the issue @vdemeester

)
configArgs = append(configArgs, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
}
cmd := repo.executor(ctx, "git", append(configArgs, args...)...)
cmd.Env = append(cmd.Environ(), env...)
Expand Down
2 changes: 1 addition & 1 deletion pkg/resolution/resolver/git/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestClone(t *testing.T) {
if test.username != "" {
token := base64.URLEncoding.EncodeToString([]byte(test.username + ":" + test.password))
expectedCmd = append(expectedCmd, "--config-env", "http.extraHeader=GIT_AUTH_HEADER")
expectedEnv = append(expectedEnv, "GIT_AUTH_HEADER=Authorization=Basic "+token)
expectedEnv = append(expectedEnv, "GIT_AUTH_HEADER=Authorization: Basic "+token)
}
expectedCmd = append(expectedCmd, "clone", test.url, repo.directory, "--depth=1", "--no-checkout")

Expand Down
56 changes: 55 additions & 1 deletion test/resolvers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,59 @@ spec:
}
}

func TestGitResolver_HTTPAuth(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

WIP regression test here

ctx := t.Context()
c, namespace := setup(ctx, t, gitFeatureFlags)

t.Parallel()

knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
defer tearDown(ctx, t, c, namespace)

giteaClusterHostname, tokenSecretName := setupGitea(ctx, t, c, namespace)

requestUrl := fmt.Sprintf("http://%s/%s/%s", net.JoinHostPort(giteaClusterHostname, "3000"), scmRemoteOrg, scmRemoteRepo)

trName := helpers.ObjectNameForTest(t)
tr := parse.MustParseV1TaskRun(t, fmt.Sprintf(`
metadata:
name: %s
namespace: %s
spec:
taskRef:
resolver: git
params:
- name: url
value: %s
- name: revision
value: %s
- name: pathInRepo
value: %s
- name: gitToken
value: %s
- name: gitTokenKey
value: %s
`,
trName,
namespace,
requestUrl,
scmRemoteBranch,
scmRemoteTaskPath,
tokenSecretName,
scmTokenSecretKey,
))

_, err := c.V1TaskRunClient.Create(ctx, tr, metav1.CreateOptions{})
if err != nil {
t.Fatalf("Failed to create TaskRun: %v", err)
}

t.Logf("Waiting for TaskRun %s in namespace %s to complete", trName, namespace)
if err := WaitForTaskRunState(ctx, c, trName, TaskRunSucceed(trName), "TaskRunSuccess", v1Version); err != nil {
t.Fatalf("Error waiting for TaskRun %s to finish: %s", trName, err)
}
}

func TestGitResolver_API(t *testing.T) {
ctx := t.Context()
c, namespace := setup(ctx, t, gitFeatureFlags)
Expand Down Expand Up @@ -699,7 +752,7 @@ spec:
},
Type: corev1.SecretTypeOpaque,
Data: map[string][]byte{
scmTokenSecretKey: []byte(base64.StdEncoding.Strict().EncodeToString([]byte(token))),
scmTokenSecretKey: []byte(token),
Comment on lines -702 to +755
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'm not sure why this was base64 encoded on the application side. When I made the gitea repo private, API auth also broke until I removed this base64 encoding. I think it was mistakenly though to be required, however the base64 encoding looks to be happening server-side after the secret is created.

},
}, metav1.CreateOptions{})

Expand Down Expand Up @@ -743,6 +796,7 @@ spec:

_, _, err = giteaClient.CreateOrgRepo(scmRemoteOrg, gitea.CreateRepoOption{
Name: scmRemoteRepo,
Private: true,
Copy link
Member

Choose a reason for hiding this comment

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

👍

AutoInit: true,
DefaultBranch: scmRemoteBranch,
})
Expand Down
Loading