diff --git a/.betterer.results b/.betterer.results index 15e0aa829f0..3be628caaeb 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5728,8 +5728,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "28"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "29"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "30"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "31"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "32"] + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "31"] ], "public/app/features/provisioning/DeleteRepositoryButton.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], diff --git a/pkg/apimachinery/go.sum b/pkg/apimachinery/go.sum index c4bc6827216..20fc926c23f 100644 --- a/pkg/apimachinery/go.sum +++ b/pkg/apimachinery/go.sum @@ -36,8 +36,6 @@ github.com/grafana/authlib v0.0.0-20250204100101-00a7f40e4029 h1:0C0a1FEkecxDk60 github.com/grafana/authlib v0.0.0-20250204100101-00a7f40e4029/go.mod h1:/gYfphsNu9v1qYWXxpv1NSvMEMSwvdf8qb8YlgwIRl8= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c h1:b0sPDtt33uFdmvUJjSCld3kwE2E49dUvevuUDSJsEuo= github.com/grafana/authlib/types v0.0.0-20250120145936-5f0e28e7a87c/go.mod h1:qYjSd1tmJiuVoSICp7Py9/zD54O9uQQA3wuM6Gg4DFM= -github.com/grafana/grafana-app-sdk/logging v0.30.0 h1:K/P/bm7Cp7Di4tqIJ3EQz2+842JozQGRaz62r95ApME= -github.com/grafana/grafana-app-sdk/logging v0.30.0/go.mod h1:xy6ZyVXl50Z3DBDLybvBPphbykPhuVNed/VNmen9DQM= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= diff --git a/pkg/apis/provisioning/v0alpha1/register.go b/pkg/apis/provisioning/v0alpha1/register.go index 6f363e47bf7..1438186062a 100644 --- a/pkg/apis/provisioning/v0alpha1/register.go +++ b/pkg/apis/provisioning/v0alpha1/register.go @@ -2,7 +2,6 @@ package v0alpha1 import ( "errors" - "fmt" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -43,7 +42,7 @@ var RepositoryResourceInfo = utils.NewResourceInfo(GROUP, VERSION, case S3RepositoryType: target = m.Spec.S3.Bucket case GitHubRepositoryType: - target = fmt.Sprintf("%s/%s", m.Spec.GitHub.Owner, m.Spec.GitHub.Repository) + target = m.Spec.GitHub.URL } return []interface{}{ diff --git a/pkg/apis/provisioning/v0alpha1/types.go b/pkg/apis/provisioning/v0alpha1/types.go index b6f42ded242..203c59d4dd3 100644 --- a/pkg/apis/provisioning/v0alpha1/types.go +++ b/pkg/apis/provisioning/v0alpha1/types.go @@ -44,27 +44,24 @@ const ( ) type GitHubRepositoryConfig struct { - // The owner of the repository (e.g. example in `example/test` or `https://github.com/example/test`). - Owner string `json:"owner,omitempty"` - // The name of the repository (e.g. test in `example/test` or `https://github.com/example/test`). - Repository string `json:"repository,omitempty"` + // The repository URL `https://github.com/example/test`). + URL string `json:"url"` + // The branch to use in the repository. // By default, this is the main branch. Branch string `json:"branch,omitempty"` + // Token for accessing the repository. // TODO: this should be part of secrets and a simple reference. Token string `json:"token,omitempty"` - // TODO: Do we want an SSH url instead maybe? - // TODO: On-prem GitHub Enterprise support? // Workflow allowed for changes to the repository. // The order is relevant for defining the precedence of the workflows. // Possible values: pull-request, branch, push. Workflows []Workflow `json:"workflows,omitempty"` - // Whether we should show dashboard previews in the pull requests caused by the BranchWorkflow option. + // Whether we should show dashboard previews for pull requests // By default, this is false (i.e. we will not create previews). - // This option is a no-op if BranchWorkflow is `false` or default. GenerateDashboardPreviews bool `json:"generateDashboardPreviews,omitempty"` } diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index dfb8863c800..ba44abc7892 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -238,16 +238,10 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref SchemaProps: spec.SchemaProps{ Type: []string{"object"}, Properties: map[string]spec.Schema{ - "owner": { + "url": { SchemaProps: spec.SchemaProps{ - Description: "The owner of the repository (e.g. example in `example/test` or `https://github.com/example/test`).", - Type: []string{"string"}, - Format: "", - }, - }, - "repository": { - SchemaProps: spec.SchemaProps{ - Description: "The name of the repository (e.g. test in `example/test` or `https://github.com/example/test`).", + Description: "The repository URL `https://github.com/example/test`).", + Default: "", Type: []string{"string"}, Format: "", }, @@ -284,12 +278,13 @@ func schema_pkg_apis_provisioning_v0alpha1_GitHubRepositoryConfig(ref common.Ref }, "generateDashboardPreviews": { SchemaProps: spec.SchemaProps{ - Description: "Whether we should show dashboard previews in the pull requests caused by the BranchWorkflow option. By default, this is false (i.e. we will not create previews). This option is a no-op if BranchWorkflow is `false` or default.", + Description: "Whether we should show dashboard previews for pull requests By default, this is false (i.e. we will not create previews).", Type: []string{"boolean"}, Format: "", }, }, }, + Required: []string{"url"}, }, }, } diff --git a/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go b/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go index e1afc1bf140..53f85b71fa8 100644 --- a/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go +++ b/pkg/generated/applyconfiguration/provisioning/v0alpha1/githubrepositoryconfig.go @@ -11,8 +11,7 @@ import ( // GitHubRepositoryConfigApplyConfiguration represents a declarative configuration of the GitHubRepositoryConfig type for use // with apply. type GitHubRepositoryConfigApplyConfiguration struct { - Owner *string `json:"owner,omitempty"` - Repository *string `json:"repository,omitempty"` + URL *string `json:"url,omitempty"` Branch *string `json:"branch,omitempty"` Token *string `json:"token,omitempty"` Workflows []provisioningv0alpha1.Workflow `json:"workflows,omitempty"` @@ -25,19 +24,11 @@ func GitHubRepositoryConfig() *GitHubRepositoryConfigApplyConfiguration { return &GitHubRepositoryConfigApplyConfiguration{} } -// WithOwner sets the Owner field in the declarative configuration to the given value +// WithURL sets the URL field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the Owner field is set to the value of the last call. -func (b *GitHubRepositoryConfigApplyConfiguration) WithOwner(value string) *GitHubRepositoryConfigApplyConfiguration { - b.Owner = &value - return b -} - -// WithRepository sets the Repository field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the Repository field is set to the value of the last call. -func (b *GitHubRepositoryConfigApplyConfiguration) WithRepository(value string) *GitHubRepositoryConfigApplyConfiguration { - b.Repository = &value +// If called multiple times, the URL field is set to the value of the last call. +func (b *GitHubRepositoryConfigApplyConfiguration) WithURL(value string) *GitHubRepositoryConfigApplyConfiguration { + b.URL = &value return b } diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 187864657f0..a3841195bb1 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -360,6 +360,11 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis r.Spec.GitHub.Branch = "main" } + // Trim trailing slash or .git + if len(r.Spec.GitHub.URL) > 5 { + r.Spec.GitHub.URL = strings.TrimRight(strings.TrimRight(r.Spec.GitHub.URL, "/"), ".git") + } + if len(r.Spec.GitHub.Workflows) == 0 { r.Spec.GitHub.Workflows = []provisioning.Workflow{ provisioning.BranchWorkflow, diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index 6679b7e1d5c..89611c06f3e 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "net/http" + "net/url" "os" "regexp" "slices" @@ -30,9 +31,12 @@ type SecretsService interface { // Make sure all public functions of this struct call the (*githubRepository).logger function, to ensure the GH repo details are included. type githubRepository struct { config *provisioning.Repository - gh pgh.Client + gh pgh.Client // assumes github.com base URL secrets SecretsService webhookURL string + + owner string + repo string } var _ Repository = (*githubRepository)(nil) @@ -44,11 +48,14 @@ func NewGitHub( secrets SecretsService, webhookURL string, ) *githubRepository { + owner, repo, _ := parseOwnerRepo(config.Spec.GitHub.URL) return &githubRepository{ config: config, - gh: factory.New(ctx, config.Spec.GitHub.Token), + gh: factory.New(ctx, config.Spec.GitHub.Token), // TODO -- base from URL secrets: secrets, webhookURL: webhookURL, + owner: owner, + repo: repo, } } @@ -63,11 +70,15 @@ func (r *githubRepository) Validate() (list field.ErrorList) { list = append(list, field.Required(field.NewPath("spec", "github"), "a github config is required")) return list } - if gh.Owner == "" { - list = append(list, field.Required(field.NewPath("spec", "github", "owner"), "a github repo owner is required")) - } - if gh.Repository == "" { - list = append(list, field.Required(field.NewPath("spec", "github", "repository"), "a github repo name is required")) + if gh.URL == "" { + list = append(list, field.Required(field.NewPath("spec", "github", "url"), "a github url is required")) + } else { + _, _, err := parseOwnerRepo(gh.URL) + if err != nil { + list = append(list, field.Invalid(field.NewPath("spec", "github", "url"), gh.URL, err.Error())) + } else if !strings.HasPrefix(gh.URL, "https://github.com/") { + list = append(list, field.Invalid(field.NewPath("spec", "github", "url"), gh.URL, "URL must start with https://github.com/")) + } } if gh.Branch == "" { list = append(list, field.Required(field.NewPath("spec", "github", "branch"), "a github branch is required")) @@ -90,6 +101,20 @@ func (r *githubRepository) Validate() (list field.ErrorList) { return list } +func parseOwnerRepo(giturl string) (owner string, repo string, err error) { + parsed, e := url.Parse(giturl) + if e != nil { + err = e + return + } + parts := strings.Split(parsed.Path, "/") + if len(parts) < 3 { + err = fmt.Errorf("unable to parse repo+owner from url") + return + } + return parts[1], parts[2], nil +} + // Test implements provisioning.Repository. func (r *githubRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { if err := r.gh.IsAuthenticated(ctx); err != nil { @@ -101,9 +126,17 @@ func (r *githubRepository) Test(ctx context.Context) (*provisioning.TestResults, }, nil } - // FIXME: check token permissions + owner, repo, err := parseOwnerRepo(r.config.Spec.GitHub.URL) + if err != nil { + return &provisioning.TestResults{ + Code: http.StatusBadRequest, + Success: false, + Errors: []string{err.Error()}, + }, nil + } - ok, err := r.gh.RepoExists(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository) + // FIXME: check token permissions + ok, err := r.gh.RepoExists(ctx, owner, repo) if err != nil { return &provisioning.TestResults{ Code: http.StatusInternalServerError, @@ -120,7 +153,7 @@ func (r *githubRepository) Test(ctx context.Context) (*provisioning.TestResults, }, nil } - ok, err = r.gh.BranchExists(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, r.config.Spec.GitHub.Branch) + ok, err = r.gh.BranchExists(ctx, r.owner, r.repo, r.config.Spec.GitHub.Branch) if err != nil { return &provisioning.TestResults{ Code: http.StatusInternalServerError, @@ -148,10 +181,8 @@ func (r *githubRepository) Read(ctx context.Context, filePath, ref string) (*Fil if ref == "" { ref = r.config.Spec.GitHub.Branch } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - content, dirContent, err := r.gh.GetContents(ctx, owner, repo, filePath, ref) + content, dirContent, err := r.gh.GetContents(ctx, r.owner, r.repo, filePath, ref) if err != nil { if errors.Is(err, pgh.ErrResourceNotFound) { return nil, &apierrors.StatusError{ @@ -187,11 +218,10 @@ func (r *githubRepository) ReadTree(ctx context.Context, ref string) ([]FileTree if ref == "" { ref = r.config.Spec.GitHub.Branch } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository + ctx, logger := r.logger(ctx, ref) - tree, truncated, err := r.gh.GetTree(ctx, owner, repo, ref, true) + tree, truncated, err := r.gh.GetTree(ctx, r.owner, r.repo, ref, true) if err != nil { if errors.Is(err, pgh.ErrResourceNotFound) { return nil, &apierrors.StatusError{ @@ -232,9 +262,6 @@ func (r *githubRepository) Create(ctx context.Context, path, ref string, data [] return fmt.Errorf("create branch on create: %w", err) } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - // Create .keep file if it is a directory if strings.HasSuffix(path, "/") { if data != nil { @@ -245,7 +272,7 @@ func (r *githubRepository) Create(ctx context.Context, path, ref string, data [] data = []byte{} } - err := r.gh.CreateFile(ctx, owner, repo, path, ref, comment, data) + err := r.gh.CreateFile(ctx, r.owner, r.repo, path, ref, comment, data) if errors.Is(err, pgh.ErrResourceAlreadyExists) { return &apierrors.StatusError{ ErrStatus: metav1.Status{ @@ -270,10 +297,7 @@ func (r *githubRepository) Update(ctx context.Context, path, ref string, data [] return fmt.Errorf("create branch on update: %w", err) } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - - file, _, err := r.gh.GetContents(ctx, owner, repo, path, ref) + file, _, err := r.gh.GetContents(ctx, r.owner, r.repo, path, ref) if err != nil { if errors.Is(err, pgh.ErrResourceNotFound) { return &apierrors.StatusError{ @@ -290,7 +314,7 @@ func (r *githubRepository) Update(ctx context.Context, path, ref string, data [] return apierrors.NewBadRequest("cannot update a directory") } - if err := r.gh.UpdateFile(ctx, owner, repo, path, ref, comment, file.GetSHA(), data); err != nil { + if err := r.gh.UpdateFile(ctx, r.owner, r.repo, path, ref, comment, file.GetSHA(), data); err != nil { return fmt.Errorf("update file: %w", err) } return nil @@ -325,10 +349,7 @@ func (r *githubRepository) Delete(ctx context.Context, path, ref, comment string } func (r *githubRepository) deleteRecursively(ctx context.Context, path, ref, comment string) error { - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - - file, contents, err := r.gh.GetContents(ctx, owner, repo, path, ref) + file, contents, err := r.gh.GetContents(ctx, r.owner, r.repo, path, ref) if err != nil { if errors.Is(err, pgh.ErrResourceNotFound) { return &apierrors.StatusError{ @@ -342,7 +363,7 @@ func (r *githubRepository) deleteRecursively(ctx context.Context, path, ref, com } if file != nil && !file.IsDirectory() { - return r.gh.DeleteFile(ctx, owner, repo, path, ref, comment, file.GetSHA()) + return r.gh.DeleteFile(ctx, r.owner, r.repo, path, ref, comment, file.GetSHA()) } for _, c := range contents { @@ -353,7 +374,7 @@ func (r *githubRepository) deleteRecursively(ctx context.Context, path, ref, com continue } - if err := r.gh.DeleteFile(ctx, owner, repo, c.GetPath(), ref, comment, c.GetSHA()); err != nil { + if err := r.gh.DeleteFile(ctx, r.owner, r.repo, c.GetPath(), ref, comment, c.GetSHA()); err != nil { return fmt.Errorf("delete file: %w", err) } } @@ -367,7 +388,7 @@ func (r *githubRepository) History(ctx context.Context, path, ref string) ([]pro } ctx, _ = r.logger(ctx, ref) - commits, err := r.gh.Commits(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, path, ref) + commits, err := r.gh.Commits(ctx, r.owner, r.repo, path, ref) if err != nil { if errors.Is(err, pgh.ErrResourceNotFound) { return nil, &apierrors.StatusError{ @@ -467,7 +488,7 @@ func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName st } } - ok, err := r.gh.BranchExists(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, branchName) + ok, err := r.gh.BranchExists(ctx, r.owner, r.repo, branchName) if err != nil { return fmt.Errorf("check branch exists: %w", err) } @@ -478,11 +499,8 @@ func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName st return nil } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - srcBranch := r.config.Spec.GitHub.Branch - if err := r.gh.CreateBranch(ctx, owner, repo, srcBranch, branchName); err != nil { + if err := r.gh.CreateBranch(ctx, r.owner, r.repo, srcBranch, branchName); err != nil { if errors.Is(err, pgh.ErrResourceAlreadyExists) { return &apierrors.StatusError{ ErrStatus: metav1.Status{ @@ -541,7 +559,7 @@ func (r *githubRepository) parsePushEvent(event *github.PushEvent) (*provisionin if event.GetRepo() == nil { return nil, fmt.Errorf("missing repository in push event") } - if event.GetRepo().GetFullName() != fmt.Sprintf("%s/%s", r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository) { + if event.GetRepo().GetFullName() != fmt.Sprintf("%s/%s", r.owner, r.repo) { return nil, fmt.Errorf("repository mismatch") } @@ -584,7 +602,7 @@ func (r *githubRepository) parsePullRequestEvent(event *github.PullRequestEvent) }, nil } - if event.GetRepo().GetFullName() != fmt.Sprintf("%s/%s", cfg.Owner, cfg.Repository) { + if event.GetRepo().GetFullName() != fmt.Sprintf("%s/%s", r.owner, r.repo) { return nil, fmt.Errorf("repository mismatch") } pr := event.GetPullRequest() @@ -626,7 +644,7 @@ func (r *githubRepository) parsePullRequestEvent(event *github.PullRequestEvent) func (r *githubRepository) LatestRef(ctx context.Context) (string, error) { ctx, _ = r.logger(ctx, "") - branch, err := r.gh.GetBranch(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, r.Config().Spec.GitHub.Branch) + branch, err := r.gh.GetBranch(ctx, r.owner, r.repo, r.Config().Spec.GitHub.Branch) if err != nil { return "", fmt.Errorf("get branch: %w", err) } @@ -644,9 +662,7 @@ func (r *githubRepository) CompareFiles(ctx context.Context, base, ref string) ( } ctx, logger := r.logger(ctx, ref) - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - files, err := r.gh.CompareCommits(ctx, owner, repo, base, ref) + files, err := r.gh.CompareCommits(ctx, r.owner, r.repo, base, ref) if err != nil { return nil, fmt.Errorf("compare commits: %w", err) } @@ -700,13 +716,13 @@ func (r *githubRepository) shouldLintPullRequest() bool { // ClearAllPullRequestFileComments clears all comments on a pull request func (r *githubRepository) ClearAllPullRequestFileComments(ctx context.Context, prNumber int) error { ctx, _ = r.logger(ctx, "") - return r.gh.ClearAllPullRequestFileComments(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber) + return r.gh.ClearAllPullRequestFileComments(ctx, r.owner, r.repo, prNumber) } // CommentPullRequest adds a comment to a pull request. func (r *githubRepository) CommentPullRequest(ctx context.Context, prNumber int, comment string) error { ctx, _ = r.logger(ctx, "") - return r.gh.CreatePullRequestComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, comment) + return r.gh.CreatePullRequestComment(ctx, r.owner, r.repo, prNumber, comment) } // CommentPullRequestFile lints a file and comments the issues found. @@ -721,7 +737,7 @@ func (r *githubRepository) CommentPullRequestFile(ctx context.Context, prNumber // FIXME: comment with Grafana Logo // FIXME: comment author should be written by Grafana and not the user - return r.gh.CreatePullRequestFileComment(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, prNumber, fileComment) + return r.gh.CreatePullRequestFileComment(ctx, r.owner, r.repo, prNumber, fileComment) } func (r *githubRepository) createWebhook(ctx context.Context) (pgh.WebhookConfig, error) { @@ -738,7 +754,7 @@ func (r *githubRepository) createWebhook(ctx context.Context) (pgh.WebhookConfig Active: true, } - hook, err := r.gh.CreateWebhook(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, cfg) + hook, err := r.gh.CreateWebhook(ctx, r.owner, r.repo, cfg) if err != nil { return pgh.WebhookConfig{}, err } @@ -758,10 +774,7 @@ func (r *githubRepository) updateWebhook(ctx context.Context) (pgh.WebhookConfig return hook, true, nil } - owner := r.config.Spec.GitHub.Owner - repoName := r.config.Spec.GitHub.Repository - - hook, err := r.gh.GetWebhook(ctx, owner, repoName, r.config.Status.Webhook.ID) + hook, err := r.gh.GetWebhook(ctx, r.owner, r.repo, r.config.Status.Webhook.ID) switch { case errors.Is(err, pgh.ErrResourceNotFound): hook, err := r.createWebhook(ctx) @@ -800,7 +813,7 @@ func (r *githubRepository) updateWebhook(ctx context.Context) (pgh.WebhookConfig return hook, false, nil } - if err := r.gh.EditWebhook(ctx, owner, repoName, hook); err != nil { + if err := r.gh.EditWebhook(ctx, r.owner, r.repo, hook); err != nil { return pgh.WebhookConfig{}, false, fmt.Errorf("edit webhook: %w", err) } @@ -815,11 +828,9 @@ func (r *githubRepository) deleteWebhook(ctx context.Context) error { return fmt.Errorf("webhook not found") } - owner := r.config.Spec.GitHub.Owner - name := r.config.Spec.GitHub.Repository id := r.config.Status.Webhook.ID - if err := r.gh.DeleteWebhook(ctx, owner, name, id); err != nil { + if err := r.gh.DeleteWebhook(ctx, r.owner, r.repo, id); err != nil { return fmt.Errorf("delete webhook: %w", err) } @@ -874,9 +885,7 @@ func (r *githubRepository) logger(ctx context.Context, ref string) (context.Cont if ref == "" { ref = r.config.Spec.GitHub.Branch } - owner := r.config.Spec.GitHub.Owner - repo := r.config.Spec.GitHub.Repository - logger = logger.With(slog.Group("github_repository", "owner", owner, "name", repo, "ref", ref)) + logger = logger.With(slog.Group("github_repository", "owner", r.owner, "name", r.repo, "ref", ref)) ctx = logging.Context(ctx, logger) // We want to ensure we don't add multiple github_repository keys. With doesn't deduplicate the keys... ctx = context.WithValue(ctx, containsGhKey, true) diff --git a/pkg/registry/apis/provisioning/repository/github_test.go b/pkg/registry/apis/provisioning/repository/github_test.go index 10cf8baa5a4..0968ffdffac 100644 --- a/pkg/registry/apis/provisioning/repository/github_test.go +++ b/pkg/registry/apis/provisioning/repository/github_test.go @@ -114,15 +114,17 @@ func TestParseWebhooks(t *testing.T) { Enabled: true, // required to accept sync job }, GitHub: &provisioning.GitHubRepositoryConfig{ - Repository: "git-ui-sync-demo", - Owner: "grafana", - Branch: "main", + URL: "https://github.com/grafana/git-ui-sync-demo", + Branch: "main", GenerateDashboardPreviews: true, }, }, }, } + var err error + gh.owner, gh.repo, err = parseOwnerRepo(gh.config.Spec.GitHub.URL) + require.NoError(t, err) for _, tt := range tests { name := fmt.Sprintf("webhook-%s-%s.json", tt.messageType, tt.name) diff --git a/pkg/registry/apis/provisioning/repository/go-git/wrapper.go b/pkg/registry/apis/provisioning/repository/go-git/wrapper.go index 048935b4924..d569fe39ffc 100644 --- a/pkg/registry/apis/provisioning/repository/go-git/wrapper.go +++ b/pkg/registry/apis/provisioning/repository/go-git/wrapper.go @@ -68,7 +68,7 @@ func Clone( return nil, err } - url := fmt.Sprintf("https://github.com/%s/%s.git", gitcfg.Owner, gitcfg.Repository) + url := fmt.Sprintf("/%s.git", gitcfg.URL) repo, err := git.PlainOpen(dir) if err != nil { diff --git a/pkg/registry/apis/provisioning/repository/go-git/wrapper_test.go b/pkg/registry/apis/provisioning/repository/go-git/wrapper_test.go index 21c01a481a9..82ee7bbae50 100644 --- a/pkg/registry/apis/provisioning/repository/go-git/wrapper_test.go +++ b/pkg/registry/apis/provisioning/repository/go-git/wrapper_test.go @@ -18,8 +18,8 @@ import ( // FIXME!! NOTE!!!!! // This is really just a sketchpad while trying to get things working // the test makes destructive changes to a real git repository :) -// this should be removed before commiting to main (likely sooner) -// and replaced with integratino tests that check the more specific results +// this should be removed before committing to main (likely sooner) +// and replaced with integration tests that check the more specific results func TestGoGitWrapper(t *testing.T) { token, ok := os.LookupEnv("gitwraptoken") if !ok { @@ -34,10 +34,9 @@ func TestGoGitWrapper(t *testing.T) { }, Spec: v0alpha1.RepositorySpec{ GitHub: &v0alpha1.GitHubRepositoryConfig{ - Owner: "grafana", - Repository: "git-ui-sync-demo", - Branch: "ryan-test", - Token: token, + URL: "https://github.com/grafana/git-ui-sync-demo", + Branch: "ryan-test", + Token: token, }, }, }, diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index 474508f1453..0c0aab6a190 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2780,27 +2780,27 @@ }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.GitHubRepositoryConfig": { "type": "object", + "required": [ + "url" + ], "properties": { "branch": { "description": "The branch to use in the repository. By default, this is the main branch.", "type": "string" }, "generateDashboardPreviews": { - "description": "Whether we should show dashboard previews in the pull requests caused by the BranchWorkflow option. By default, this is false (i.e. we will not create previews). This option is a no-op if BranchWorkflow is `false` or default.", + "description": "Whether we should show dashboard previews for pull requests By default, this is false (i.e. we will not create previews).", "type": "boolean" }, - "owner": { - "description": "The owner of the repository (e.g. example in `example/test` or `https://github.com/example/test`).", - "type": "string" - }, - "repository": { - "description": "The name of the repository (e.g. test in `example/test` or `https://github.com/example/test`).", - "type": "string" - }, "token": { "description": "Token for accessing the repository.", "type": "string" }, + "url": { + "description": "The repository URL `https://github.com/example/test`).", + "type": "string", + "default": "" + }, "workflows": { "description": "Workflow allowed for changes to the repository. The order is relevant for defining the precedence of the workflows. Possible values: pull-request, branch, push.", "type": "array", diff --git a/pkg/tests/apis/provisioning/testdata/github-example.json b/pkg/tests/apis/provisioning/testdata/github-example.json index 03d7dc8b124..85475eb8613 100644 --- a/pkg/tests/apis/provisioning/testdata/github-example.json +++ b/pkg/tests/apis/provisioning/testdata/github-example.json @@ -15,8 +15,7 @@ "intervalSeconds": 60 }, "github": { - "owner": "grafana", - "repository": "git-ui-sync-demo", + "url": "https://github.com/grafana/git-ui-sync-demo", "branch": "dummy-branch", "workflows": ["push", "branch"], "generateDashboardPreviews": true, diff --git a/public/app/features/provisioning/ConfigForm.tsx b/public/app/features/provisioning/ConfigForm.tsx index a594fbebaa7..00d8b338ba0 100644 --- a/public/app/features/provisioning/ConfigForm.tsx +++ b/public/app/features/provisioning/ConfigForm.tsx @@ -44,8 +44,7 @@ function getDefaultValues(repository?: RepositorySpec): RepositoryFormData { type: 'github', title: '', token: '', - owner: '', - repository: '', + url: '', branch: 'main', generateDashboardPreviews: true, workflows: ['push', 'branch'], @@ -153,13 +152,25 @@ export function ConfigForm({ data }: ConfigFormProps) { /> - - + + - - - - + diff --git a/public/app/features/provisioning/RepositoryListPage.tsx b/public/app/features/provisioning/RepositoryListPage.tsx index a4cd88fac65..aa00bea056c 100644 --- a/public/app/features/provisioning/RepositoryListPage.tsx +++ b/public/app/features/provisioning/RepositoryListPage.tsx @@ -72,7 +72,7 @@ function RepositoryListPageContent({ items }: { items?: Repository[] }) { case 'github': icon = 'github'; const spec = item.spec.github; - const url = `https://github.com/${spec?.owner}/${spec?.repository}/`; + const url = item.spec.github?.url ?? ''; let branch = url; if (spec?.branch) { branch += `tree/` + spec?.branch; diff --git a/public/app/features/provisioning/RepositoryOverview.tsx b/public/app/features/provisioning/RepositoryOverview.tsx index 996da9a371e..fea093d319f 100644 --- a/public/app/features/provisioning/RepositoryOverview.tsx +++ b/public/app/features/provisioning/RepositoryOverview.tsx @@ -154,9 +154,9 @@ export function RepositoryOverview({ repo }: { repo: Repository }) { function getRemoteURL(repo: Repository) { if (repo.spec?.type === 'github') { const spec = repo.spec.github; - let url = `https://github.com/${spec?.owner}/${spec?.repository}/`; + let url = spec?.url || ''; if (spec?.branch) { - url += `tree/${spec.branch}`; + url += `/tree/${spec.branch}`; } return url; } @@ -165,9 +165,8 @@ function getRemoteURL(repo: Repository) { function getWebhookURL(repo: Repository) { const { status, spec } = repo; - if (spec?.type === 'github' && status?.webhook?.url) { - const { github } = spec; - return `https://github.com/${github?.owner}/${github?.repository}/settings/hooks/${status.webhook?.id}`; + if (spec?.type === 'github' && status?.webhook?.url && spec.github?.url) { + return `${spec.github.url}/settings/hooks/${status.webhook?.id}`; } return undefined; } diff --git a/public/app/features/provisioning/api/endpoints.gen.ts b/public/app/features/provisioning/api/endpoints.gen.ts index 3855ecbf299..0f65b3622be 100644 --- a/public/app/features/provisioning/api/endpoints.gen.ts +++ b/public/app/features/provisioning/api/endpoints.gen.ts @@ -736,14 +736,12 @@ export type JobList = { export type GitHubRepositoryConfig = { /** The branch to use in the repository. By default, this is the main branch. */ branch?: string; - /** Whether we should show dashboard previews in the pull requests caused by the BranchWorkflow option. By default, this is false (i.e. we will not create previews). This option is a no-op if BranchWorkflow is `false` or default. */ + /** Whether we should show dashboard previews for pull requests By default, this is false (i.e. we will not create previews). */ generateDashboardPreviews?: boolean; - /** The owner of the repository (e.g. example in `example/test` or `https://github.com/example/test`). */ - owner?: string; - /** The name of the repository (e.g. test in `example/test` or `https://github.com/example/test`). */ - repository?: string; /** Token for accessing the repository. */ token?: string; + /** The repository URL `https://github.com/example/test`). */ + url: string; /** Workflow allowed for changes to the repository. The order is relevant for defining the precedence of the workflows. Possible values: pull-request, branch, push. */ workflows?: ('branch' | 'push')[]; }; diff --git a/public/app/features/provisioning/utils/data.ts b/public/app/features/provisioning/utils/data.ts index 1cc5ecc6845..d50ea74fe6e 100644 --- a/public/app/features/provisioning/utils/data.ts +++ b/public/app/features/provisioning/utils/data.ts @@ -13,8 +13,7 @@ export const dataToSpec = (data: RepositoryFormData): RepositorySpec => { spec.github = { workflows: data.workflows, generateDashboardPreviews: data.generateDashboardPreviews, - owner: data.owner, - repository: data.repository, + url: data.url || '', branch: data.branch, token: data.token, }; @@ -38,10 +37,9 @@ export const dataToSpec = (data: RepositoryFormData): RepositorySpec => { export const specToData = (spec: RepositorySpec): RepositoryFormData => { return { ...spec, - owner: spec?.github?.owner || '', - repository: spec?.github?.repository || '', ...spec.github, ...spec.local, ...spec.s3, + url: spec.github?.url || '', }; }; diff --git a/public/app/features/provisioning/utils/git.ts b/public/app/features/provisioning/utils/git.ts index 6905e69d418..1e89f106a5e 100644 --- a/public/app/features/provisioning/utils/git.ts +++ b/public/app/features/provisioning/utils/git.ts @@ -4,7 +4,7 @@ export function createPRLink(spec?: RepositorySpec, dashboardName?: string, ref? if (!spec || spec.type !== 'github' || !ref) { return ''; } - return `https://github.com/${spec.github?.owner}/${spec.github?.repository}/compare/${spec.github?.branch}...${ref}?quick_pull=1&labels=grafana&title=Update dashboard ${dashboardName}&body=${encodeURI(comment || '')}`; + return `${spec.github?.url}/compare/${spec.github?.branch}...${ref}?quick_pull=1&labels=grafana&title=Update dashboard ${dashboardName}&body=${encodeURI(comment || '')}`; } /**