Provisioning: handle .git extension more gracefully (#108213)

* Move .git to repository packages

* Bump nanogit 2025-07-17

This version handles `.git` extension internally so that the client
doesn't have to worry about it

* Put back mutation for Github

* Mutate also git URL for clarity
pull/106267/merge
Roberto Jiménez Sánchez 2 days ago committed by GitHub
parent 446054a61d
commit 4847882ee7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      go.mod
  2. 4
      go.sum
  3. 35
      pkg/registry/apis/provisioning/register.go
  4. 21
      pkg/registry/apis/provisioning/repository/git/mutator.go
  5. 127
      pkg/registry/apis/provisioning/repository/git/mutator_test.go
  6. 10
      pkg/registry/apis/provisioning/repository/github/mutator.go
  7. 68
      pkg/registry/apis/provisioning/repository/github/mutator_test.go
  8. 5
      pkg/registry/apis/provisioning/repository/github/repository.go

@ -104,7 +104,7 @@ require (
github.com/grafana/grafana-openapi-client-go v0.0.0-20231213163343-bd475d63fb79 // @grafana/grafana-backend-group
github.com/grafana/grafana-plugin-sdk-go v0.278.0 // @grafana/plugins-platform-backend
github.com/grafana/loki/v3 v3.2.1 // @grafana/observability-logs
github.com/grafana/nanogit v0.0.0-20250709085038-55508a6a9f40 // @grafana-app-platform-squad
github.com/grafana/nanogit v0.0.0-20250717084510-7027b3f0138e // @grafana-app-platform-squad
github.com/grafana/otel-profiling-go v0.5.1 // @grafana/grafana-backend-group
github.com/grafana/pyroscope-go/godeltaprof v0.1.8 // @grafana/observability-traces-and-profiling
github.com/grafana/pyroscope/api v1.2.1-0.20250415190842-3ff7247547ae // @grafana/observability-traces-and-profiling

@ -1641,8 +1641,8 @@ github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 h1:ZYk42718k
github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608/go.mod h1:f3JSoxBTPXX5ec4FxxeC19nTBSxoTz+cBgS3cYLMcr0=
github.com/grafana/loki/v3 v3.2.1 h1:VB7u+KHfvL5aHAxgoVBvz5wVhsdGuqKC7uuOFOOe7jw=
github.com/grafana/loki/v3 v3.2.1/go.mod h1:WvdLl6wOS+yahaeQY+xhD2m2XzkHDfKr5FZaX7D/X2Y=
github.com/grafana/nanogit v0.0.0-20250709085038-55508a6a9f40 h1:wsIgOI4Ou1o/UtxtJlemLufpVBpMdcXVJxedk0wLoCM=
github.com/grafana/nanogit v0.0.0-20250709085038-55508a6a9f40/go.mod h1:ToqLjIdvV3AZQa3K6e5m9hy/nsGaUByc2dWQlctB9iA=
github.com/grafana/nanogit v0.0.0-20250717084510-7027b3f0138e h1:IcrC8SqJcNbGlNq0nqptJ4X8pyy21smMaFrhA7DrfYg=
github.com/grafana/nanogit v0.0.0-20250717084510-7027b3f0138e/go.mod h1:ToqLjIdvV3AZQa3K6e5m9hy/nsGaUByc2dWQlctB9iA=
github.com/grafana/otel-profiling-go v0.5.1 h1:stVPKAFZSa7eGiqbYuG25VcqYksR6iWvF3YH66t4qL8=
github.com/grafana/otel-profiling-go v0.5.1/go.mod h1:ftN/t5A/4gQI19/8MoWurBEtC6gFw8Dns1sJZ9W4Tls=
github.com/grafana/prometheus-alertmanager v0.25.1-0.20250620093340-be61a673dee6 h1:oJnbhG6ZNy10AjsgNeAtAKeGHogIGOMfAsBH6fYYa5M=

@ -438,41 +438,6 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis
r.Spec.Sync.IntervalSeconds = 60
}
// TODO: move this logic into github repository concrete implementation.
if r.Spec.Type == provisioning.GitHubRepositoryType {
if r.Spec.GitHub == nil {
return fmt.Errorf("github configuration is required")
}
// Trim trailing slash or .git
if len(r.Spec.GitHub.URL) > 5 {
r.Spec.GitHub.URL = strings.TrimSuffix(r.Spec.GitHub.URL, ".git")
r.Spec.GitHub.URL = strings.TrimSuffix(r.Spec.GitHub.URL, "/")
}
}
if r.Spec.Type == provisioning.GitRepositoryType {
if r.Spec.Git == nil {
return fmt.Errorf("git configuration is required")
}
if r.Spec.GitHub != nil {
return fmt.Errorf("git and github cannot be used together")
}
if r.Spec.Local != nil {
return fmt.Errorf("git and local cannot be used together")
}
// Trim trailing slash and ensure .git is present
if len(r.Spec.Git.URL) > 5 {
r.Spec.Git.URL = strings.TrimSuffix(r.Spec.Git.URL, "/")
if !strings.HasSuffix(r.Spec.Git.URL, ".git") {
r.Spec.Git.URL = r.Spec.Git.URL + ".git"
}
}
}
if r.Spec.Workflows == nil {
r.Spec.Workflows = []provisioning.Workflow{}
}

@ -2,6 +2,8 @@ package git
import (
"context"
"fmt"
"strings"
"k8s.io/apimachinery/pkg/runtime"
@ -17,10 +19,27 @@ func Mutator(secrets secrets.RepositorySecrets) controller.Mutator {
return nil
}
if repo.Spec.Git == nil {
if repo.Spec.Type != provisioning.GitRepositoryType {
return nil
}
if repo.Spec.Git == nil {
return fmt.Errorf("git configuration is required for git repository type")
}
if repo.Spec.Git.URL != "" {
url := strings.TrimSpace(repo.Spec.Git.URL)
if url != "" {
// Remove any trailing slashes
url = strings.TrimRight(url, "/")
// Only add .git if it's not already present
if !strings.HasSuffix(url, ".git") {
url = url + ".git"
}
repo.Spec.Git.URL = url
}
}
if repo.Spec.Git.Token != "" {
secretName := repo.Name + gitTokenSecretSuffix
nameOrValue, err := secrets.Encrypt(ctx, repo, secretName, repo.Spec.Git.Token)

@ -21,6 +21,7 @@ func TestMutator(t *testing.T) {
expectedToken string
expectedEncryptedToken string
expectedError string
expectedURL string
}{
{
name: "successful token encryption",
@ -30,6 +31,7 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
Token: "secret-token",
},
@ -44,6 +46,7 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
Token: "secret-token",
},
@ -64,6 +67,7 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
Token: "secret-token",
},
@ -78,6 +82,7 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
Token: "secret-token",
},
@ -97,13 +102,32 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Git: nil,
Type: provisioning.LocalRepositoryType,
Git: nil,
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
},
{
name: "no git spec for git repository type",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: nil,
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedError: "git configuration is required for git repository type",
},
{
name: "empty token",
obj: &provisioning.Repository{
@ -112,6 +136,7 @@ func TestMutator(t *testing.T) {
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
Token: "",
},
@ -128,6 +153,101 @@ func TestMutator(t *testing.T) {
// No expectations
},
},
{
name: "URL normalization - add .git suffix",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
URL: "https://github.com/grafana/grafana",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedURL: "https://github.com/grafana/grafana.git",
},
{
name: "URL normalization - keep existing .git suffix",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
URL: "https://github.com/grafana/grafana.git",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedURL: "https://github.com/grafana/grafana.git",
},
{
name: "URL normalization - remove trailing slash and add .git",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
URL: "https://github.com/grafana/grafana/",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedURL: "https://github.com/grafana/grafana.git",
},
{
name: "URL normalization - trim whitespace and add .git",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
URL: " https://github.com/grafana/grafana ",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedURL: "https://github.com/grafana/grafana.git",
},
{
name: "URL normalization - empty URL after trim",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
Type: provisioning.GitRepositoryType,
Git: &provisioning.GitRepositoryConfig{
URL: " ",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
// No expectations
},
expectedURL: "",
},
}
for _, tt := range tests {
@ -152,6 +272,11 @@ func TestMutator(t *testing.T) {
// EncryptedToken should be set to the expected value
assert.Equal(t, tt.expectedEncryptedToken, string(repo.Spec.Git.EncryptedToken), "EncryptedToken should match expected value")
}
// Check URL normalization
if tt.expectedURL != "" {
assert.Equal(t, tt.expectedURL, repo.Spec.Git.URL, "URL should be normalized correctly")
}
}
}
})

@ -2,6 +2,7 @@ package github
import (
"context"
"strings"
"k8s.io/apimachinery/pkg/runtime"
@ -21,6 +22,15 @@ func Mutator(secrets secrets.RepositorySecrets) controller.Mutator {
return nil
}
// Trim trailing ".git" and any trailing slash from the GitHub URL, if present, using the strings package.
if repo.Spec.GitHub.URL != "" {
url := repo.Spec.GitHub.URL
url = strings.TrimRight(url, "/")
url = strings.TrimSuffix(url, ".git")
url = strings.TrimRight(url, "/")
repo.Spec.GitHub.URL = url
}
if repo.Spec.GitHub.Token != "" {
secretName := repo.Name + githubTokenSecretSuffix
nameOrValue, err := secrets.Encrypt(ctx, repo, secretName, repo.Spec.GitHub.Token)

@ -22,6 +22,74 @@ func TestMutator(t *testing.T) {
expectedEncryptedToken string
expectedError string
}{
{
name: "trims trailing .git and slash from GitHub URL",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo1",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
URL: "https://github.com/org/repo.git/",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {},
expectedToken: "",
expectedEncryptedToken: "",
},
{
name: "trims only trailing slash from GitHub URL",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo2",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
URL: "https://github.com/org/repo/",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {},
expectedToken: "",
expectedEncryptedToken: "",
},
{
name: "trims only trailing .git from GitHub URL",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo3",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
URL: "https://github.com/org/repo.git",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {},
expectedToken: "",
expectedEncryptedToken: "",
},
{
name: "does not trim if no .git or slash",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "repo4",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
URL: "https://github.com/org/repo",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {},
expectedToken: "",
expectedEncryptedToken: "",
},
{
name: "successful token encryption",
obj: &provisioning.Repository{

@ -114,7 +114,10 @@ func (r *githubRepository) Validate() (list field.ErrorList) {
}
func ParseOwnerRepoGithub(giturl string) (owner string, repo string, err error) {
parsed, e := url.Parse(strings.TrimSuffix(giturl, ".git"))
giturl = strings.TrimSuffix(giturl, ".git")
giturl = strings.TrimSuffix(giturl, "/")
parsed, e := url.Parse(giturl)
if e != nil {
err = e
return

Loading…
Cancel
Save