diff --git a/cmd/controller_cmd.go b/cmd/controller_cmd.go index fc3c77c3..886c9831 100644 --- a/cmd/controller_cmd.go +++ b/cmd/controller_cmd.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "fmt" "os" "os/signal" @@ -15,6 +16,7 @@ import ( "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/config" "github.com/zapier/kubechecks/pkg/events" + "github.com/zapier/kubechecks/pkg/repo" "github.com/zapier/kubechecks/pkg/server" ) @@ -24,13 +26,28 @@ var ControllerCmd = &cobra.Command{ Short: "Start the VCS Webhook handler.", Long: ``, Run: func(cmd *cobra.Command, args []string) { - fmt.Println("Starting KubeChecks:", pkg.GitTag, pkg.GitCommit) + clientType := viper.GetString("vcs-type") + client, err := createVCSClient(clientType) + if err != nil { + log.Fatal().Err(err).Msg("failed to create vcs client") + } - server := server.NewServer(&config.ServerConfig{ + cfg := config.ServerConfig{ UrlPrefix: viper.GetString("webhook-url-prefix"), WebhookSecret: viper.GetString("webhook-secret"), - }) - go server.Start() + VcsClient: client, + } + + log.Info().Msg("Initializing git settings") + if err := repo.InitializeGitSettings(cfg.VcsClient.Username(), cfg.VcsClient.Email()); err != nil { + log.Fatal().Err(err).Msg("failed to initialize git settings") + } + + fmt.Println("Starting KubeChecks:", pkg.GitTag, pkg.GitCommit) + server := server.NewServer(&cfg) + + ctx := context.Background() + go server.Start(ctx) // graceful termination handler. // when we receive a SIGTERM from kubernetes, check for in-flight requests before exiting. diff --git a/cmd/process.go b/cmd/process.go new file mode 100644 index 00000000..bb9336c6 --- /dev/null +++ b/cmd/process.go @@ -0,0 +1,52 @@ +package cmd + +import ( + "context" + + "github.com/rs/zerolog/log" + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/zapier/kubechecks/pkg/config" + "github.com/zapier/kubechecks/pkg/server" +) + +var processCmd = &cobra.Command{ + Use: "process", + Short: "Process a pull request", + Long: "", + Run: func(cmd *cobra.Command, args []string) { + ctx := context.TODO() + + log.Info().Msg("building apps map from argocd") + result, err := config.BuildAppsMap(ctx) + if err != nil { + log.Fatal().Err(err).Msg("failed to build apps map") + } + + clientType := viper.GetString("vcs-type") + client, err := createVCSClient(clientType) + if err != nil { + log.Fatal().Err(err).Msg("failed to create vcs client") + } + + cfg := config.ServerConfig{ + UrlPrefix: "--unused--", + WebhookSecret: "--unused--", + VcsToArgoMap: result, + VcsClient: client, + } + + repo, err := client.LoadHook(ctx, args[0]) + if err != nil { + log.Fatal().Err(err).Msg("failed to load hook") + return + } + + server.ProcessCheckEvent(ctx, repo, &cfg) + }, +} + +func init() { + RootCmd.AddCommand(processCmd) +} diff --git a/cmd/vcs.go b/cmd/vcs.go new file mode 100644 index 00000000..5cad3db7 --- /dev/null +++ b/cmd/vcs.go @@ -0,0 +1,20 @@ +package cmd + +import ( + "fmt" + + "github.com/zapier/kubechecks/pkg/vcs" + "github.com/zapier/kubechecks/pkg/vcs/github_client" + "github.com/zapier/kubechecks/pkg/vcs/gitlab_client" +) + +func createVCSClient(clientType string) (vcs.Client, error) { + switch clientType { + case "gitlab": + return gitlab_client.CreateGitlabClient() + case "github": + return github_client.CreateGithubClient() + default: + return nil, fmt.Errorf("unknown vcs type: %s", clientType) + } +} diff --git a/pkg/affected_apps/best_effort.go b/pkg/affected_apps/best_effort.go deleted file mode 100644 index 9a709b6a..00000000 --- a/pkg/affected_apps/best_effort.go +++ /dev/null @@ -1,124 +0,0 @@ -package affected_apps - -import ( - "context" - "fmt" - "path/filepath" - "strings" - - "github.com/rs/zerolog/log" - - "github.com/zapier/kubechecks/pkg/config" -) - -var KustomizeSubPaths = []string{"base/", "bases/", "components/", "overlays/", "resources/"} - -type BestEffort struct { - repoName string - repoFileList []string -} - -func NewBestEffortMatcher(repoName string, repoFileList []string) *BestEffort { - return &BestEffort{ - repoName: repoName, - repoFileList: repoFileList, - } -} - -func (b *BestEffort) AffectedApps(_ context.Context, changeList []string, targetBranch string) (AffectedItems, error) { - appsMap := make(map[string]string) - - for _, file := range changeList { - fileParts := strings.Split(file, "/") - // Expected structure is /apps// or /manifests/ - // and thus anything shorter than 3 elements isn't an Argo manifest - if len(fileParts) < 3 { - continue - } - // If using the /apps/ pattern, the application name is cluster-app from the fileparts - if fileParts[0] == "apps" { - if isKustomizeApp(file) { - if isKustomizeBaseComponentsChange(file) { - // return all apps in overlays dir adjacent to the change dir - oversDir := overlaysDir(file) - for _, repoFile := range b.repoFileList { - if strings.Contains(repoFile, oversDir) { - repoFileParts := strings.Split(repoFile, "/") - appName := fmt.Sprintf("%s-%s", repoFileParts[3], fileParts[1]) - appPath := fmt.Sprintf("%s%s/", oversDir, repoFileParts[3]) - log.Debug().Str("app", appName).Str("path", appPath).Msg("adding application to map") - appsMap[appName] = appPath - } - } - } else { - appsMap[fmt.Sprintf("%s-%s", fileParts[3], fileParts[1])] = fmt.Sprintf("%s/%s/%s/%s/", fileParts[0], fileParts[1], fileParts[2], fileParts[3]) - } - } else { - // helm - if isHelmClusterAppFile(file) { - appsMap[fmt.Sprintf("%s-%s", fileParts[2], fileParts[1])] = fmt.Sprintf("%s/%s/%s/", fileParts[0], fileParts[1], fileParts[2]) - } else { - // touching a file that is at the helm root, return list of all cluster apps below this dir - appDir := filepath.Dir(file) - for _, repoFile := range b.repoFileList { - dir := filepath.Dir(repoFile) - if dir != appDir && strings.Contains(dir, appDir) { - repoFileParts := strings.Split(dir, "/") - if len(repoFileParts) > 2 && len(fileParts) > 1 { - appName := fmt.Sprintf("%s-%s", repoFileParts[2], fileParts[1]) - appPath := fmt.Sprintf("%s/%s/", appDir, repoFileParts[2]) - log.Debug().Str("app", appName).Str("path", appPath).Msg("adding application to map") - appsMap[appName] = appPath - } else { - log.Warn().Str("dir", dir).Msg("ignoring dir") - } - } - } - } - } - } - // If using the /manifests/ pattern, we need the repo name to use as the app - if fileParts[0] == "manifests" || fileParts[0] == "charts" { - appsMap[fmt.Sprintf("%s-%s", fileParts[1], b.repoName)] = fmt.Sprintf("%s/%s/", fileParts[0], fileParts[1]) - } - } - - var appsSlice []config.ApplicationStub - for name, path := range appsMap { - appsSlice = append(appsSlice, config.ApplicationStub{Name: name, Path: path}) - } - - return AffectedItems{Applications: appsSlice}, nil -} - -func isHelmClusterAppFile(file string) bool { - dir := filepath.Dir(file) - return len(strings.Split(dir, "/")) > 2 -} - -func isKustomizeApp(file string) bool { - if file == "kustomization.yaml" { - return true - } else { - for _, sub := range KustomizeSubPaths { - if strings.Contains(file, sub) { - return true - } - } - } - return false -} - -func isKustomizeBaseComponentsChange(file string) bool { - return strings.Contains(file, "base/") || - strings.Contains(file, "bases/") || - strings.Contains(file, "components/") || - strings.Contains(file, "resources/") -} - -func overlaysDir(file string) string { - appBaseDir := filepath.Dir(filepath.Dir(file)) - overlays := filepath.Join(appBaseDir, "overlays/") - - return overlays + "/" -} diff --git a/pkg/affected_apps/best_effort_test.go b/pkg/affected_apps/best_effort_test.go deleted file mode 100644 index 9f0702cd..00000000 --- a/pkg/affected_apps/best_effort_test.go +++ /dev/null @@ -1,320 +0,0 @@ -package affected_apps - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/zapier/kubechecks/pkg/config" -) - -func TestBestEffortMatcher(t *testing.T) { - type args struct { - fileList []string - repoName string - } - tests := []struct { - name string - args args - want AffectedItems - }{ - { - name: "helm:cluster-change", - args: args{ - fileList: []string{ - "apps/echo-server/foo-eks-01/values.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-echo-server", Path: "apps/echo-server/foo-eks-01/"}, - }, - }, - }, - { - name: "helm:all-cluster-change", - args: args{ - fileList: []string{ - "apps/echo-server/values.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-echo-server", Path: "apps/echo-server/foo-eks-01/"}, - {Name: "foo-eks-02-echo-server", Path: "apps/echo-server/foo-eks-02/"}, - }, - }, - }, - { - name: "helm:all-cluster-change:and:cluster-app-change", - args: args{ - fileList: []string{ - "apps/echo-server/values.yaml", - "apps/echo-server/foo-eks-01/values.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-echo-server", Path: "apps/echo-server/foo-eks-01/"}, - {Name: "foo-eks-02-echo-server", Path: "apps/echo-server/foo-eks-02/"}, - }, - }, - }, - { - name: "helm:all-cluster-change:and:double-cluster-app-change", - args: args{ - fileList: []string{ - "apps/echo-server/values.yaml", - "apps/echo-server/foo-eks-01/values.yaml", - "apps/echo-server/foo-eks-02/values.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-echo-server", Path: "apps/echo-server/foo-eks-01/"}, - {Name: "foo-eks-02-echo-server", Path: "apps/echo-server/foo-eks-02/"}, - }, - }, - }, - { - name: "kustomize:overlays-change", - args: args{ - fileList: []string{ - "apps/httpbin/overlays/foo-eks-01/kustomization.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-httpbin", Path: "apps/httpbin/overlays/foo-eks-01/"}, - }, - }, - }, - { - name: "kustomize:overlays-subdir-change", - args: args{ - fileList: []string{ - "apps/httpbin/overlays/foo-eks-01/server/deploy.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-httpbin", Path: "apps/httpbin/overlays/foo-eks-01/"}, - }, - }, - }, - { - name: "kustomize:base-change", - args: args{ - fileList: []string{ - "apps/httpbin/base/kustomization.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-httpbin", Path: "apps/httpbin/overlays/foo-eks-01/"}, - }, - }, - }, - { - name: "kustomize:bases-change", - args: args{ - fileList: []string{ - "apps/httpbin/bases/foo.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-httpbin", Path: "apps/httpbin/overlays/foo-eks-01/"}, - }, - }, - }, - { - name: "kustomize:resources-change", - args: args{ - fileList: []string{ - "apps/httpbin/resources/foo.yaml", - }, - repoName: "", - }, - want: AffectedItems{ - Applications: []config.ApplicationStub{ - {Name: "foo-eks-01-httpbin", Path: "apps/httpbin/overlays/foo-eks-01/"}, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var got AffectedItems - var err error - - matcher := NewBestEffortMatcher(tt.args.repoName, testRepoFiles) - got, err = matcher.AffectedApps(context.TODO(), tt.args.fileList, "master") - require.NoError(t, err) - - assert.Equal(t, len(tt.want.Applications), len(got.Applications)) - assert.Equal(t, len(tt.want.ApplicationSets), len(got.ApplicationSets)) - - // ordering doesn't matter, we just want to make sure the items all exist - wantAppsMap := listToMap(tt.want.Applications, appStubKey) - gotAppsMap := listToMap(got.Applications, appStubKey) - assert.Equal(t, wantAppsMap, gotAppsMap, "Applications not equal") - - wantAppSetsMap := listToMap(tt.want.ApplicationSets, appSetKey) - gotAppSetsMap := listToMap(got.ApplicationSets, appSetKey) - assert.Equal(t, wantAppSetsMap, gotAppSetsMap, "ApplicationSets not equal") - }) - } -} - -func appSetKey(item ApplicationSet) string { - return item.Name -} - -func appStubKey(stub config.ApplicationStub) string { - return stub.Name -} - -func listToMap[T any](items []T, makeKey func(T) string) map[string]T { - result := make(map[string]T) - for _, item := range items { - key := makeKey(item) - result[key] = item - } - return result -} - -var testRepoFiles = []string{ - "apps/echo-server/foo-eks-01/Chart.yaml", - "apps/echo-server/foo-eks-01/values.yaml", - "apps/echo-server/foo-eks-01/templates/something.yaml", - "apps/echo-server/foo-eks-02/Chart.yaml", - "apps/echo-server/foo-eks-02/values.yaml", - "apps/echo-server/foo-eks-02/templates/something.yaml", - "apps/echo-server/values.yaml", - "apps/echo-server/opslevel.yml", - "apps/httpbin/base/kustomization.yaml", - "apps/httpbin/bases/deploy.yaml", - "apps/httpbin/resources/configmap.yaml", - "apps/httpbin/overlays/foo-eks-01/kustomization.yaml", - "apps/httpbin/overlays/foo-eks-01/server/deploy.yaml", - "apps/httpbin/components/kustomization.yaml", -} - -func Test_isKustomizeApp(t *testing.T) { - type args struct { - file string - } - tests := []struct { - name string - args args - want bool - }{ - { - "overlayskustomzation.yaml", - args{ - "apps/foo/overlays/kustomization.yaml", - }, - true, - }, - { - "basekustomzation.yaml", - args{ - "apps/foo/overlays/kustomization.yaml", - }, - true, - }, - { - "overlaysfile", - args{ - "apps/foo/overlays/foo.yaml", - }, - true, - }, - { - "basefile", - args{ - "apps/foo/base/foo.yaml", - }, - true, - }, - { - "helmvalues", - args{ - "apps/foo/values.yaml", - }, - false, - }, - { - "helmclustervalues", - args{ - "apps/foo/cluster/values.yaml", - }, - false, - }, - { - "helmvalues", - args{ - "apps/foo/values.yaml", - }, - false, - }, - { - "basesfile", - args{ - "apps/foo/bases/foo.yaml", - }, - true, - }, - { - "resourcesfile", - args{ - "apps/foo/resources/foo.yaml", - }, - true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := isKustomizeApp(tt.args.file); got != tt.want { - t.Errorf("isKustomizeApp() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_overlaysDir(t *testing.T) { - type args struct { - file string - } - tests := []struct { - name string - args args - want string - }{ - { - "basic", - args{ - file: "apps/foo/base/kustomization.yaml", - }, - "apps/foo/overlays/", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := overlaysDir(tt.args.file); got != tt.want { - t.Errorf("overlaysDir() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/affected_apps/config_matcher.go b/pkg/affected_apps/config_matcher.go index f7ef4aba..b0f0091e 100644 --- a/pkg/affected_apps/config_matcher.go +++ b/pkg/affected_apps/config_matcher.go @@ -6,10 +6,11 @@ import ( "path" "strings" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/pkg/errors" "github.com/rs/zerolog/log" "github.com/zapier/kubechecks/pkg/argo_client" - "github.com/zapier/kubechecks/pkg/config" "github.com/zapier/kubechecks/pkg/repo_config" ) @@ -24,7 +25,7 @@ func NewConfigMatcher(cfg *repo_config.Config) *ConfigMatcher { } func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) { - appsMap := make(map[string]string) + triggeredAppsMap := make(map[string]string) var appSetList []ApplicationSet triggeredApps, triggeredAppsets, err := b.triggeredApps(ctx, changeList) @@ -33,19 +34,28 @@ func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, t } for _, app := range triggeredApps { - appsMap[app.Name] = app.Path + triggeredAppsMap[app.Name] = app.Path } for _, appset := range triggeredAppsets { appSetList = append(appSetList, ApplicationSet{appset.Name}) } - var appsSlice []config.ApplicationStub - for name, appPath := range appsMap { - appsSlice = append(appsSlice, config.ApplicationStub{Name: name, Path: appPath}) + allArgoApps, err := b.argoClient.GetApplications(ctx) + if err != nil { + return AffectedItems{}, errors.Wrap(err, "failed to list applications") + } + + var triggeredAppsSlice []v1alpha1.Application + for _, app := range allArgoApps.Items { + if _, ok := triggeredAppsMap[app.Name]; !ok { + continue + } + + triggeredAppsSlice = append(triggeredAppsSlice, app) } - return AffectedItems{Applications: appsSlice, ApplicationSets: appSetList}, nil + return AffectedItems{Applications: triggeredAppsSlice, ApplicationSets: appSetList}, nil } func (b *ConfigMatcher) triggeredApps(ctx context.Context, modifiedFiles []string) ([]*repo_config.ArgoCdApplicationConfig, []*repo_config.ArgocdApplicationSetConfig, error) { diff --git a/pkg/affected_apps/matcher.go b/pkg/affected_apps/matcher.go index 9928ad88..42427759 100644 --- a/pkg/affected_apps/matcher.go +++ b/pkg/affected_apps/matcher.go @@ -4,11 +4,11 @@ import ( "context" "path" - "github.com/zapier/kubechecks/pkg/config" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) type AffectedItems struct { - Applications []config.ApplicationStub + Applications []v1alpha1.Application ApplicationSets []ApplicationSet } diff --git a/pkg/argo_client/applications.go b/pkg/argo_client/applications.go index 598b866e..4203b383 100644 --- a/pkg/argo_client/applications.go +++ b/pkg/argo_client/applications.go @@ -12,8 +12,9 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/pkg/errors" - "github.com/zapier/kubechecks/telemetry" "go.opentelemetry.io/otel" + + "github.com/zapier/kubechecks/telemetry" ) // GetApplicationByName takes a context and a name, then queries the Argo Application client to retrieve the Application with the specified name. @@ -35,23 +36,16 @@ func (argo *ArgoClient) GetApplicationByName(ctx context.Context, name string) ( return resp, nil } -// GetKubernetesVersionByApplicationName is a method on the ArgoClient struct that takes a context and an application name as parameters, +// GetKubernetesVersionByApplication is a method on the ArgoClient struct that takes a context and an application name as parameters, // and returns the Kubernetes version of the destination cluster where the specified application is running. // It returns an error if the application or cluster information cannot be retrieved. -func (argo *ArgoClient) GetKubernetesVersionByApplicationName(ctx context.Context, appName string) (string, error) { +func (argo *ArgoClient) GetKubernetesVersionByApplication(ctx context.Context, app v1alpha1.Application) (string, error) { ctx, span := otel.Tracer("Kubechecks").Start(ctx, "GetKubernetesVersionByApplicationName") defer span.End() - // Get application - app, err := argo.GetApplicationByName(ctx, appName) - if err != nil { - telemetry.SetError(span, err, "Argo Get Application By Name error") - return "", err - } - // Get destination cluster // Some app specs have a Name defined, some have a Server defined, some have both, take a valid one and use it - log.Debug().Msgf("for appname %s, server dest says: %s and name dest says: %s", appName, app.Spec.Destination.Server, app.Spec.Destination.Name) + log.Debug().Msgf("for appname %s, server dest says: %s and name dest says: %s", app.Name, app.Spec.Destination.Server, app.Spec.Destination.Name) var clusterRequest *cluster.ClusterQuery if app.Spec.Destination.Server != "" { clusterRequest = &cluster.ClusterQuery{Server: app.Spec.Destination.Server} diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 3e99b4dc..d61a54a9 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster" "github.com/argoproj/argo-cd/v2/pkg/apiclient/settings" argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" @@ -13,13 +12,17 @@ import ( "github.com/argoproj/argo-cd/v2/reposerver/repository" "github.com/argoproj/argo-cd/v2/util/git" "github.com/ghodss/yaml" + "github.com/pkg/errors" "github.com/rs/zerolog/log" - "github.com/zapier/kubechecks/telemetry" "go.opentelemetry.io/otel" "k8s.io/apimachinery/pkg/api/resource" + + "github.com/zapier/kubechecks/telemetry" ) -func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, changedAppFilePath string) ([]string, error) { +func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, changedAppFilePath string, app argoappv1.Application) ([]string, error) { + var err error + ctx, span := otel.Tracer("Kubechecks").Start(ctx, "GetManifestsLocal") defer span.End() @@ -32,40 +35,28 @@ func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, cha }() argoClient := GetArgoClient() - appCloser, appClient := argoClient.GetApplicationClient() - defer appCloser.Close() - - clusterCloser, clusterIf := argoClient.GetClusterClient() + clusterCloser, clusterClient := argoClient.GetClusterClient() defer clusterCloser.Close() settingsCloser, settingsClient := argoClient.GetSettingsClient() defer settingsCloser.Close() - log.Debug().Str("name", name).Msg("generating diff for application...") - - appName := name - app, err := appClient.Get(ctx, &application.ApplicationQuery{ - Name: &appName, - }) - if err != nil { - telemetry.SetError(span, err, "Argo Get App") - getManifestsFailed.WithLabelValues(name).Inc() - return nil, err - } - log.Trace().Msgf("Argo App: %+v", app) - - cluster, err := clusterIf.Get(ctx, &cluster.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server}) + log.Debug(). + Str("clusterName", app.Spec.Destination.Name). + Str("clusterServer", app.Spec.Destination.Server). + Msg("getting cluster") + cluster, err := clusterClient.Get(ctx, &cluster.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server}) if err != nil { telemetry.SetError(span, err, "Argo Get Cluster") getManifestsFailed.WithLabelValues(name).Inc() - return nil, err + return nil, errors.Wrap(err, "failed to get cluster") } argoSettings, err := settingsClient.Get(ctx, &settings.SettingsQuery{}) if err != nil { telemetry.SetError(span, err, "Argo Get Settings") getManifestsFailed.WithLabelValues(name).Inc() - return nil, err + return nil, errors.Wrap(err, "failed to get settings") } // Code is commented out until Argo fixes the server side manifest generation @@ -81,8 +72,8 @@ func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, cha */ source := app.Spec.GetSource() - log.Debug().Msgf("App source: %+v", source) + log.Debug().Str("name", name).Msg("generating diff for application...") res, err := repository.GenerateManifests(ctx, fmt.Sprintf("%s/%s", tempRepoDir, changedAppFilePath), tempRepoDir, source.TargetRevision, &repoapiclient.ManifestRequest{ Repo: &argoappv1.Repository{Repo: source.RepoURL}, AppLabelKey: argoSettings.AppLabelKey, @@ -97,7 +88,7 @@ func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, cha }, true, &git.NoopCredsStore{}, resource.MustParse("0"), nil) if err != nil { telemetry.SetError(span, err, "Generate Manifests") - return nil, err + return nil, errors.Wrap(err, "failed to generate manifests") } if res.Manifests == nil { @@ -108,7 +99,7 @@ func GetManifestsLocal(ctx context.Context, name string, tempRepoDir string, cha } func FormatManifestsYAML(manifestBytes []string) []string { - manifests := []string{} + var manifests []string for _, manifest := range manifestBytes { ret, err := yaml.JSONToYAML([]byte(manifest)) if err != nil { diff --git a/pkg/config/app_directory.go b/pkg/config/app_directory.go index 98053fcf..603e6fe5 100644 --- a/pkg/config/app_directory.go +++ b/pkg/config/app_directory.go @@ -8,24 +8,18 @@ import ( "github.com/rs/zerolog/log" ) -type ApplicationStub struct { - Name, Path, TargetRevision string - - IsHelm, IsKustomize bool -} - type AppDirectory struct { appDirs map[string][]string // directory -> array of app names appFiles map[string][]string // file path -> array of app names - appsMap map[string]ApplicationStub // app name -> app stub + appsMap map[string]v1alpha1.Application // app name -> app stub } func NewAppDirectory() *AppDirectory { return &AppDirectory{ appDirs: make(map[string][]string), appFiles: make(map[string][]string), - appsMap: make(map[string]ApplicationStub), + appsMap: make(map[string]v1alpha1.Application), } } @@ -35,7 +29,7 @@ func (d *AppDirectory) Count() int { func (d *AppDirectory) Union(other *AppDirectory) *AppDirectory { var join AppDirectory - join.appsMap = mergeMaps(d.appsMap, other.appsMap, takeFirst[ApplicationStub]) + join.appsMap = mergeMaps(d.appsMap, other.appsMap, takeFirst[v1alpha1.Application]) join.appDirs = mergeMaps(d.appDirs, other.appDirs, mergeLists[string]) join.appFiles = mergeMaps(d.appFiles, other.appFiles, mergeLists[string]) return &join @@ -44,14 +38,11 @@ func (d *AppDirectory) Union(other *AppDirectory) *AppDirectory { func (d *AppDirectory) ProcessApp(app v1alpha1.Application) { appName := app.Name - src := app.Spec.Source - if src == nil { - return - } + src := app.Spec.GetSource() // common data srcPath := src.Path - d.AddAppStub(appName, srcPath, app.Spec.Source.TargetRevision, src.IsHelm(), !src.Kustomize.IsZero()) + d.AddApp(app) // handle extra helm paths if helm := src.Helm; helm != nil { @@ -67,7 +58,7 @@ func (d *AppDirectory) ProcessApp(app v1alpha1.Application) { } } -func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBranch string) []ApplicationStub { +func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBranch string) []v1alpha1.Application { log.Debug().Msgf("checking %d changes", len(changeList)) appsSet := make(map[string]struct{}) @@ -75,7 +66,6 @@ func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBran log.Debug().Msgf("change: %s", changePath) for dir, appNames := range d.appDirs { - log.Debug().Msgf("- app path: %s", dir) if strings.HasPrefix(changePath, dir) { log.Debug().Msg("dir match!") for _, appName := range appNames { @@ -93,7 +83,7 @@ func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBran } } - var appsSlice []ApplicationStub + var appsSlice []v1alpha1.Application for appName := range appsSet { app, ok := d.appsMap[appName] if !ok { @@ -102,7 +92,7 @@ func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBran } if !shouldInclude(app, targetBranch) { - log.Debug().Msgf("target revision of %s is %s and does not match '%s'", appName, app.TargetRevision, targetBranch) + log.Debug().Msgf("target revision of %s is %s and does not match '%s'", appName, getTargetRevision(app), targetBranch) continue } @@ -113,16 +103,25 @@ func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBran return appsSlice } -func shouldInclude(app ApplicationStub, targetBranch string) bool { - if app.TargetRevision == "" { +func getTargetRevision(app v1alpha1.Application) string { + return app.Spec.GetSource().TargetRevision +} + +func getSourcePath(app v1alpha1.Application) string { + return app.Spec.GetSource().Path +} + +func shouldInclude(app v1alpha1.Application, targetBranch string) bool { + targetRevision := getTargetRevision(app) + if targetRevision == "" { return true } - if app.TargetRevision == targetBranch { + if targetRevision == targetBranch { return true } - if app.TargetRevision == "HEAD" { + if targetRevision == "HEAD" { if targetBranch == "main" { return true } @@ -135,8 +134,8 @@ func shouldInclude(app ApplicationStub, targetBranch string) bool { return false } -func (d *AppDirectory) GetApps(filter func(stub ApplicationStub) bool) []ApplicationStub { - var result []ApplicationStub +func (d *AppDirectory) GetApps(filter func(stub v1alpha1.Application) bool) []v1alpha1.Application { + var result []v1alpha1.Application for _, value := range d.appsMap { if filter != nil && !filter(value) { continue @@ -146,15 +145,14 @@ func (d *AppDirectory) GetApps(filter func(stub ApplicationStub) bool) []Applica return result } -func (d *AppDirectory) AddAppStub(appName, srcPath, targetRevision string, isHelm, isKustomize bool) { - d.appsMap[appName] = ApplicationStub{ - Name: appName, - TargetRevision: targetRevision, - Path: srcPath, - IsHelm: isHelm, - IsKustomize: isKustomize, - } - d.AddDir(appName, srcPath) +func (d *AppDirectory) AddApp(app v1alpha1.Application) { + log.Debug(). + Str("appName", app.Name). + Str("cluster-name", app.Spec.Destination.Name). + Str("cluster-server", app.Spec.Destination.Server). + Msg("found app") + d.appsMap[app.Name] = app + d.AddDir(app.Name, getSourcePath(app)) } func (d *AppDirectory) AddDir(appName, path string) { diff --git a/pkg/config/app_directory_test.go b/pkg/config/app_directory_test.go index afe2a094..8453d677 100644 --- a/pkg/config/app_directory_test.go +++ b/pkg/config/app_directory_test.go @@ -32,11 +32,8 @@ func TestPathsAreJoinedProperly(t *testing.T) { rad.ProcessApp(app1) - assert.Equal(t, map[string]ApplicationStub{ - "test-app": { - Name: "test-app", - Path: "/test1/test2", - }, + assert.Equal(t, map[string]v1alpha1.Application{ + "test-app": app1, }, rad.appsMap) assert.Equal(t, map[string][]string{ "/test1/test2": {"test-app"}, @@ -91,7 +88,13 @@ func TestShouldInclude(t *testing.T) { for _, tc := range testcases { t.Run(fmt.Sprintf("%v", tc), func(t *testing.T) { - actual := shouldInclude(ApplicationStub{TargetRevision: tc.argocdAppBranch}, tc.vcsMergeTarget) + actual := shouldInclude(v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + TargetRevision: tc.argocdAppBranch, + }, + }, + }, tc.vcsMergeTarget) assert.Equal(t, tc.expected, actual) }) } diff --git a/pkg/config/config.go b/pkg/config/config.go index c6edec28..ace5b34b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -8,6 +8,8 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/rs/zerolog/log" giturls "github.com/whilp/git-urls" + + "github.com/zapier/kubechecks/pkg/vcs" ) type repoURL struct { @@ -55,6 +57,7 @@ type ServerConfig struct { UrlPrefix string WebhookSecret string VcsToArgoMap VcsToArgoMap + VcsClient vcs.Client } func (cfg *ServerConfig) GetVcsRepos() []string { diff --git a/pkg/config/vcstoargomap.go b/pkg/config/vcstoargomap.go index 5a1e7d19..47e82f22 100644 --- a/pkg/config/vcstoargomap.go +++ b/pkg/config/vcstoargomap.go @@ -1,10 +1,13 @@ package config import ( + "context" "io/fs" + "github.com/pkg/errors" "github.com/rs/zerolog/log" + "github.com/zapier/kubechecks/pkg/argo_client" "github.com/zapier/kubechecks/pkg/repo" ) @@ -18,6 +21,21 @@ func NewVcsToArgoMap() VcsToArgoMap { } } +func BuildAppsMap(ctx context.Context) (VcsToArgoMap, error) { + result := NewVcsToArgoMap() + argoClient := argo_client.GetArgoClient() + + apps, err := argoClient.GetApplications(ctx) + if err != nil { + return result, errors.Wrap(err, "failed to list applications") + } + for _, app := range apps.Items { + result.AddApp(app) + } + + return result, nil +} + func (v2a *VcsToArgoMap) GetAppsInRepo(repoCloneUrl string) *AppDirectory { repoUrl, err := normalizeRepoUrl(repoCloneUrl) if err != nil { @@ -43,8 +61,9 @@ func (v2a *VcsToArgoMap) WalkKustomizeApps(repo *repo.Repo, fs fs.FS) *AppDirect ) for _, app := range apps { - if err = walkKustomizeFiles(result, fs, app.Name, app.Path); err != nil { - log.Error().Err(err).Msgf("failed to parse kustomize.yaml in %s", app.Path) + appPath := app.Spec.GetSource().Path + if err = walkKustomizeFiles(result, fs, app.Name, appPath); err != nil { + log.Error().Err(err).Msgf("failed to parse kustomize.yaml in %s", appPath) } } diff --git a/pkg/config/walk_kustomize_files_test.go b/pkg/config/walk_kustomize_files_test.go index 0a2bef6a..3834fc50 100644 --- a/pkg/config/walk_kustomize_files_test.go +++ b/pkg/config/walk_kustomize_files_test.go @@ -4,8 +4,10 @@ import ( "testing" "testing/fstest" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestKustomizeWalking(t *testing.T) { @@ -73,10 +75,32 @@ resources: } ) + newApp := func(name, path, revision string, isHelm, isKustomize bool) v1alpha1.Application { + app := v1alpha1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + Path: path, + TargetRevision: revision, + }, + }, + } + + if isHelm { + app.Spec.Source.Helm = &v1alpha1.ApplicationSourceHelm{} + } + if isKustomize { + app.Spec.Source.Kustomize = &v1alpha1.ApplicationSourceKustomize{} + } + return app + } + appdir := NewAppDirectory() - appdir.AddAppStub(kustomizeApp1Name, kustomizeApp1Path, "HEAD", false, true) - appdir.AddAppStub(kustomizeApp2Name, kustomizeApp2Path, "HEAD", false, true) - appdir.AddAppStub(kustomizeBaseName, kustomizeBasePath, "HEAD", false, true) + appdir.AddApp(newApp(kustomizeApp1Name, kustomizeApp1Path, "HEAD", false, true)) + appdir.AddApp(newApp(kustomizeApp2Name, kustomizeApp2Path, "HEAD", false, true)) + appdir.AddApp(newApp(kustomizeBaseName, kustomizeBasePath, "HEAD", false, true)) err = walkKustomizeFiles(appdir, fs, kustomizeApp1Name, kustomizeApp1Path) require.NoError(t, err) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index fd55672d..efc198de 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -17,7 +17,6 @@ import ( "github.com/argoproj/gitops-engine/pkg/sync/hook" "github.com/argoproj/gitops-engine/pkg/sync/ignore" "github.com/argoproj/gitops-engine/pkg/utils/kube" - "github.com/argoproj/pkg/errors" "github.com/ghodss/yaml" "github.com/go-logr/zerologr" "github.com/pmezard/go-difflib/difflib" @@ -38,6 +37,10 @@ type objKeyLiveTarget struct { target *unstructured.Unstructured } +func isAppMissingErr(err error) bool { + return strings.Contains(err.Error(), "PermissionDenied") +} + /* Take cli output and return as a string or an array of strings instead of printing @@ -45,45 +48,41 @@ changedFilePath should be the root of the changed folder from https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L879 */ -func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckResult, string, error) { - ctx, span := otel.Tracer("Kubechecks").Start(ctx, "Diff") +func GetDiff(ctx context.Context, manifests []string, app argoappv1.Application, addApp func(argoappv1.Application)) (pkg.CheckResult, string, error) { + ctx, span := otel.Tracer("Kubechecks").Start(ctx, "GetDiff") defer span.End() argoClient := argo_client.GetArgoClient() - closer, appClient := argoClient.GetApplicationClient() - log.Debug().Str("name", name).Msg("generating diff for application...") - - defer closer.Close() + log.Debug().Str("name", app.Name).Msg("generating diff for application...") settingsCloser, settingsClient := argoClient.GetSettingsClient() defer settingsCloser.Close() - var err error - - appName := name - app, err := appClient.Get(ctx, &application.ApplicationQuery{ - Name: &appName, - }) - if err != nil { - telemetry.SetError(span, err, "Get Argo App") - return pkg.CheckResult{}, "", err - } + closer, appClient := argoClient.GetApplicationClient() + defer closer.Close() resources, err := appClient.ManagedResources(ctx, &application.ResourcesQuery{ - ApplicationName: &appName, + ApplicationName: &app.Name, }) if err != nil { - telemetry.SetError(span, err, "Get Argo Managed Resources") - return pkg.CheckResult{}, "", err + if !isAppMissingErr(err) { + telemetry.SetError(span, err, "Get Argo Managed Resources") + return pkg.CheckResult{}, "", err + } + + resources = new(application.ManagedResourcesResponse) } - errors.CheckError(err) items := make([]objKeyLiveTarget, 0) var unstructureds []*unstructured.Unstructured for _, mfst := range manifests { obj, err := argoappv1.UnmarshalToUnstructured(mfst) - errors.CheckError(err) + if err != nil { + log.Warn().Err(err).Msg("failed to unmarshal to unstructured") + continue + } + unstructureds = append(unstructureds, obj) } argoSettings, err := settingsClient.Get(ctx, &settings.SettingsQuery{}) @@ -98,8 +97,15 @@ func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckRes return pkg.CheckResult{}, "", err } - groupedObjs := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace) - items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.Name) + groupedObjs, err := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace) + if err != nil { + return pkg.CheckResult{}, "", err + } + + if items, err = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.Name); err != nil { + return pkg.CheckResult{}, "", err + } + diffBuffer := &strings.Builder{} var added, modified, removed int for _, item := range items { @@ -123,9 +129,16 @@ func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckRes WithTracking(argoSettings.AppLabelKey, argoSettings.TrackingMethod). WithNoCache(). Build() - errors.CheckError(err) + if err != nil { + telemetry.SetError(span, err, "Build Diff") + return pkg.CheckResult{}, "failed to build diff", err + } + diffRes, err := argodiff.StateDiff(item.live, item.target, diffConfig) - errors.CheckError(err) + if err != nil { + telemetry.SetError(span, err, "State Diff") + return pkg.CheckResult{}, "failed to state diff", err + } if diffRes.Modified || item.target == nil || item.live == nil { diffBuffer.WriteString(fmt.Sprintf("===== %s ======\n", resourceId)) @@ -134,8 +147,10 @@ func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckRes if item.target != nil && item.live != nil { target = &unstructured.Unstructured{} live = item.live - err = json.Unmarshal(diffRes.PredictedLive, target) - errors.CheckError(err) + if err = json.Unmarshal(diffRes.PredictedLive, target); err != nil { + telemetry.SetError(span, err, "JSON Unmarshall") + log.Warn().Err(err).Msg("failed to unmarshall json") + } } else { live = item.live target = item.target @@ -151,6 +166,9 @@ func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckRes removed++ case item.live == nil: added++ + if app, ok := isApp(item, diffRes.PredictedLive); ok { + addApp(app) + } case diffRes.Modified: modified++ } @@ -172,8 +190,29 @@ func GetDiff(ctx context.Context, name string, manifests []string) (pkg.CheckRes return cr, diff, nil } +var nilApp = argoappv1.Application{} + +func isApp(item objKeyLiveTarget, manifests []byte) (argoappv1.Application, bool) { + if strings.ToLower(item.key.Group) != "argoproj.io" { + log.Debug().Str("group", item.key.Group).Msg("group is not correct") + return nilApp, false + } + if strings.ToLower(item.key.Kind) != "application" { + log.Debug().Str("kind", item.key.Kind).Msg("kind is not correct") + return nilApp, false + } + + var app argoappv1.Application + if err := json.Unmarshal(manifests, &app); err != nil { + log.Warn().Err(err).Msg("failed to deserialize application") + return nilApp, false + } + + return app, true +} + // from https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L879 -func groupObjsByKey(localObs []*unstructured.Unstructured, liveObjs []*unstructured.Unstructured, appNamespace string) map[kube.ResourceKey]*unstructured.Unstructured { +func groupObjsByKey(localObs []*unstructured.Unstructured, liveObjs []*unstructured.Unstructured, appNamespace string) (map[kube.ResourceKey]*unstructured.Unstructured, error) { namespacedByGk := make(map[schema.GroupKind]bool) for i := range liveObjs { if liveObjs[i] != nil { @@ -182,7 +221,10 @@ func groupObjsByKey(localObs []*unstructured.Unstructured, liveObjs []*unstructu } } localObs, _, err := controller.DeduplicateTargetObjects(appNamespace, localObs, &resourceInfoProvider{namespacedByGk: namespacedByGk}) - errors.CheckError(err) + if err != nil { + return nil, err + } + objByKey := make(map[kube.ResourceKey]*unstructured.Unstructured) for i := range localObs { obj := localObs[i] @@ -190,16 +232,17 @@ func groupObjsByKey(localObs []*unstructured.Unstructured, liveObjs []*unstructu objByKey[kube.GetResourceKey(obj)] = obj } } - return objByKey + return objByKey, nil } // from https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L879 -func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[kube.ResourceKey]*unstructured.Unstructured, items []objKeyLiveTarget, argoSettings *settings.Settings, appName string) []objKeyLiveTarget { +func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[kube.ResourceKey]*unstructured.Unstructured, items []objKeyLiveTarget, argoSettings *settings.Settings, appName string) ([]objKeyLiveTarget, error) { resourceTracking := argo.NewResourceTracking() for _, res := range resources.Items { var live = &unstructured.Unstructured{} - err := json.Unmarshal([]byte(res.NormalizedLiveState), &live) - errors.CheckError(err) + if err := json.Unmarshal([]byte(res.NormalizedLiveState), &live); err != nil { + return nil, err + } key := kube.ResourceKey{Name: res.Name, Namespace: res.Namespace, Group: res.Group, Kind: res.Kind} if key.Kind == kube.SecretKind && key.Group == "" { @@ -209,8 +252,9 @@ func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[ } if local, ok := objs[key]; ok || live != nil { if local != nil && !kube.IsCRD(local) { - err = resourceTracking.SetAppInstance(local, argoSettings.AppLabelKey, appName, "", argoappv1.TrackingMethod(argoSettings.GetTrackingMethod())) - errors.CheckError(err) + if err := resourceTracking.SetAppInstance(local, argoSettings.AppLabelKey, appName, "", argoappv1.TrackingMethod(argoSettings.GetTrackingMethod())); err != nil { + return nil, err + } } items = append(items, objKeyLiveTarget{key, live, local}) @@ -225,7 +269,8 @@ func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[ } items = append(items, objKeyLiveTarget{key, nil, local}) } - return items + + return items, nil } // from https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L879 diff --git a/pkg/events/check.go b/pkg/events/check.go index fc72d7ba..ce4495d8 100644 --- a/pkg/events/check.go +++ b/pkg/events/check.go @@ -9,6 +9,7 @@ import ( "sync/atomic" "time" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -27,13 +28,13 @@ import ( "github.com/zapier/kubechecks/pkg/repo" "github.com/zapier/kubechecks/pkg/repo_config" "github.com/zapier/kubechecks/pkg/validate" + "github.com/zapier/kubechecks/pkg/vcs" "github.com/zapier/kubechecks/telemetry" ) type CheckEvent struct { - client pkg.Client // Client exposing methods to communicate with platform of user choice + client vcs.Client // Client exposing methods to communicate with platform of user choice fileList []string // What files have changed in this PR/MR - repoFiles []string // All files in this repository TempWorkingDir string // Location of the local repo repo *repo.Repo logger zerolog.Logger @@ -43,14 +44,18 @@ type CheckEvent struct { affectedItems affected_apps.AffectedItems cfg *config.ServerConfig + + addedAppsSet map[string]struct{} + appChannel chan *v1alpha1.Application + doneChannel chan bool } var inFlight int32 -func NewCheckEvent(repo *repo.Repo, client pkg.Client, cfg *config.ServerConfig) *CheckEvent { +func NewCheckEvent(repo *repo.Repo, cfg *config.ServerConfig) *CheckEvent { ce := &CheckEvent{ cfg: cfg, - client: client, + client: cfg.VcsClient, repo: repo, } @@ -93,14 +98,6 @@ func (ce *CheckEvent) Cleanup(ctx context.Context) { } } -// InitializeGit sets the username and email for a git repo -func (ce *CheckEvent) InitializeGit(ctx context.Context) error { - _, span := otel.Tracer("Kubechecks").Start(ctx, "InitializeGit") - defer span.End() - - return repo.InitializeGitSettings(ce.repo.Username, ce.repo.Email) -} - // CloneRepoLocal takes the repo inside the Check Event and try to clone it locally func (ce *CheckEvent) CloneRepoLocal(ctx context.Context) error { _, span := otel.Tracer("Kubechecks").Start(ctx, "CloneRepoLocal") @@ -152,23 +149,12 @@ func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context, targetBran if cfg != nil { log.Debug().Msg("using the config matcher") matcher = affected_apps.NewConfigMatcher(cfg) - } else if viper.GetBool("monitor-all-applications") { + } else { log.Debug().Msg("using an argocd matcher") matcher, err = affected_apps.NewArgocdMatcher(ce.cfg.VcsToArgoMap, ce.repo, ce.TempWorkingDir) if err != nil { return errors.Wrap(err, "failed to create argocd matcher") } - } else { - log.Debug().Msg("using best effort matcher") - ce.repoFiles, err = ce.repo.GetListOfRepoFiles() - if err != nil { - telemetry.SetError(span, err, "Get List of Repo Files") - - ce.logger.Error().Err(err).Msg("could not get list of repo files") - // continue with an empty list - ce.repoFiles = []string{} - } - matcher = affected_apps.NewBestEffortMatcher(ce.repo.Name, ce.repoFiles) } ce.affectedItems, err = matcher.AffectedApps(ctx, ce.fileList, targetBranch) if err != nil { @@ -187,11 +173,6 @@ func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context, targetBran return err } -type appStruct struct { - name string - dir string -} - func (ce *CheckEvent) ProcessApps(ctx context.Context) { ctx, span := otel.Tracer("Kubechecks").Start(ctx, "ProcessApps", trace.WithAttributes( @@ -213,8 +194,9 @@ func (ce *CheckEvent) ProcessApps(ctx context.Context) { } // Concurrently process all apps, with a corresponding error channel for reporting back failures - appChannel := make(chan appStruct, len(ce.affectedItems.Applications)) - doneChannel := make(chan bool, len(ce.affectedItems.Applications)) + ce.addedAppsSet = make(map[string]struct{}) + ce.appChannel = make(chan *v1alpha1.Application, len(ce.affectedItems.Applications)*2) + ce.doneChannel = make(chan bool, len(ce.affectedItems.Applications)*2) // If the number of affected apps that we have is less than our worker limit, lower the worker limit if ce.workerLimits > len(ce.affectedItems.Applications) { @@ -225,36 +207,36 @@ func (ce *CheckEvent) ProcessApps(ctx context.Context) { ce.vcsNote = ce.createNote(ctx) for w := 0; w <= ce.workerLimits; w++ { - go ce.appWorkers(ctx, w, appChannel, doneChannel) + go ce.appWorkers(ctx, w) } // Produce apps onto channel for _, app := range ce.affectedItems.Applications { - a := appStruct{ - name: app.Name, - dir: app.Path, - } - ce.logger.Trace().Str("app", a.name).Str("dir", a.dir).Msg("producing app on channel") - appChannel <- a + ce.queueApp(app) } returnCount := 0 commitStatus := true - for appStatus := range doneChannel { + for appStatus := range ce.doneChannel { + ce.logger.Debug().Msg("finished an app") if !appStatus { + ce.logger.Debug().Msg("app failed, commit status = false") commitStatus = false } returnCount++ - if returnCount == len(ce.affectedItems.Applications) { + ce.logger.Debug().Int("done apps", returnCount).Int("all apps", len(ce.addedAppsSet)).Msg("completed apps") + + if returnCount == len(ce.addedAppsSet) { ce.logger.Debug().Msg("Closing channels") - close(appChannel) - close(doneChannel) + close(ce.appChannel) + close(ce.doneChannel) } } ce.logger.Info().Msg("Finished") - if err = ce.vcsNote.PushComment(ctx, ce.client); err != nil { + comment := ce.vcsNote.BuildComment(ctx) + if err = ce.client.UpdateMessage(ctx, ce.vcsNote, comment); err != nil { ce.logger.Error().Err(err).Msg("failed to push comment") } @@ -267,6 +249,27 @@ func (ce *CheckEvent) ProcessApps(ctx context.Context) { ce.CommitStatus(ctx, pkg.StateSuccess) } +func (ce *CheckEvent) queueApp(app v1alpha1.Application) { + name := app.Name + dir := app.Spec.GetSource().Path + + if _, ok := ce.addedAppsSet[name]; ok { + return + } + + ce.addedAppsSet[name] = struct{}{} + + logger := ce.logger.Debug(). + Str("app", name). + Str("dir", dir). + Str("cluster-name", app.Spec.Destination.Name). + Str("cluster-server", app.Spec.Destination.Server) + + logger.Msg("producing app on channel") + ce.appChannel <- &app + logger.Msg("finished producing app") +} + // CommitStatus sets the commit status on the MR // To set the PR/MR status func (ce *CheckEvent) CommitStatus(ctx context.Context, status pkg.CommitState) { @@ -279,11 +282,18 @@ func (ce *CheckEvent) CommitStatus(ctx context.Context, status pkg.CommitState) } // Process all apps on the provided channel -func (ce *CheckEvent) appWorkers(ctx context.Context, workerID int, appChannel chan appStruct, resultChannel chan bool) { - for app := range appChannel { - ce.logger.Info().Int("workerID", workerID).Str("app", app.name).Msg("Processing App") - isSuccess := ce.processApp(ctx, app.name, app.dir) - resultChannel <- isSuccess +func (ce *CheckEvent) appWorkers(ctx context.Context, workerID int) { + for app := range ce.appChannel { + var isSuccess bool + + if app != nil { + ce.logger.Info().Int("workerID", workerID).Str("app", app.Name).Msg("Processing App") + isSuccess = ce.processApp(ctx, *app) + } else { + log.Warn().Msg("appWorkers received a nil app") + } + + ce.doneChannel <- isSuccess } } @@ -292,9 +302,12 @@ func (ce *CheckEvent) appWorkers(ctx context.Context, workerID int, appChannel c // It takes a context (ctx), application name (app), directory (dir) as input and returns an error if any check fails. // The processing is performed concurrently using Go routines and error groups. Any check results are sent through // the returnChan. The function also manages the inFlight atomic counter to track active processing routines. -func (ce *CheckEvent) processApp(ctx context.Context, app, dir string) bool { +func (ce *CheckEvent) processApp(ctx context.Context, app v1alpha1.Application) bool { + appName := app.Name + dir := app.Spec.GetSource().Path + ctx, span := otel.Tracer("Kubechecks").Start(ctx, "processApp", trace.WithAttributes( - attribute.String("app", app), + attribute.String("app", appName), attribute.String("dir", dir), )) defer span.End() @@ -303,16 +316,16 @@ func (ce *CheckEvent) processApp(ctx context.Context, app, dir string) bool { defer atomic.AddInt32(&inFlight, -1) start := time.Now() - ce.logger.Info().Str("app", app).Msg("Adding new app") + ce.logger.Info().Str("app", appName).Msg("Adding new app") // Build a new section for this app in the parent comment - ce.vcsNote.AddNewApp(ctx, app) + ce.vcsNote.AddNewApp(ctx, appName) - ce.logger.Debug().Msgf("Getting manifests for app: %s with code at %s/%s", app, ce.TempWorkingDir, dir) - manifests, err := argo_client.GetManifestsLocal(ctx, app, ce.TempWorkingDir, dir) + ce.logger.Debug().Msgf("Getting manifests for app: %s with code at %s/%s", appName, ce.TempWorkingDir, dir) + manifests, err := argo_client.GetManifestsLocal(ctx, appName, ce.TempWorkingDir, dir, app) if err != nil { - ce.logger.Error().Err(err).Msgf("Unable to get manifests for %s in %s", app, dir) + ce.logger.Error().Err(err).Msgf("Unable to get manifests for %s in %s", appName, dir) cr := pkg.CheckResult{State: pkg.StateError, Summary: "Unable to get manifests", Details: fmt.Sprintf("```\n%s\n```", ce.cleanupGetManifestsError(err))} - ce.vcsNote.AddToAppMessage(ctx, app, cr) + ce.vcsNote.AddToAppMessage(ctx, appName, cr) return false } @@ -320,7 +333,7 @@ func (ce *CheckEvent) processApp(ctx context.Context, app, dir string) bool { formattedManifests := argo_client.FormatManifestsYAML(manifests) ce.logger.Trace().Msgf("Manifests:\n%+v\n", formattedManifests) - k8sVersion, err := argo_client.GetArgoClient().GetKubernetesVersionByApplicationName(ctx, app) + k8sVersion, err := argo_client.GetArgoClient().GetKubernetesVersionByApplication(ctx, app) if err != nil { ce.logger.Error().Err(err).Msg("Error retrieving the Kubernetes version") k8sVersion = viper.GetString("fallback-k8s-version") @@ -331,16 +344,16 @@ func (ce *CheckEvent) processApp(ctx context.Context, app, dir string) bool { var wg sync.WaitGroup - run := ce.createRunner(span, ctx, app, &wg) + run := ce.createRunner(span, ctx, appName, &wg) - run("validating app against schema", ce.validateSchemas(ctx, app, k8sVersion, ce.TempWorkingDir, formattedManifests)) - run("generating diff for app", ce.generateDiff(ctx, app, manifests)) + run("validating app against schema", ce.validateSchemas(ctx, appName, k8sVersion, ce.TempWorkingDir, formattedManifests)) + run("generating diff for app", ce.generateDiff(ctx, app, manifests, ce.queueApp)) if viper.GetBool("enable-conftest") { - run("validation policy", ce.validatePolicy(ctx, app)) + run("validation policy", ce.validatePolicy(ctx, appName)) } - run("running pre-upgrade check", ce.runPreupgradeCheck(ctx, app, k8sVersion, formattedManifests)) + run("running pre-upgrade check", ce.runPreupgradeCheck(ctx, appName, k8sVersion, formattedManifests)) wg.Wait() @@ -424,14 +437,14 @@ func (ce *CheckEvent) validatePolicy(ctx context.Context, app string) func() (pk } } -func (ce *CheckEvent) generateDiff(ctx context.Context, app string, manifests []string) func() (pkg.CheckResult, error) { +func (ce *CheckEvent) generateDiff(ctx context.Context, app v1alpha1.Application, manifests []string, addApp func(app v1alpha1.Application)) func() (pkg.CheckResult, error) { return func() (pkg.CheckResult, error) { - cr, rawDiff, err := diff.GetDiff(ctx, app, manifests) + cr, rawDiff, err := diff.GetDiff(ctx, manifests, app, addApp) if err != nil { return pkg.CheckResult{}, err } - diff.AIDiffSummary(ctx, ce.vcsNote, app, manifests, rawDiff) + diff.AIDiffSummary(ctx, ce.vcsNote, app.Name, manifests, rawDiff) return cr, nil } diff --git a/pkg/github_client/client_test.go b/pkg/github_client/client_test.go deleted file mode 100644 index 7513ddca..00000000 --- a/pkg/github_client/client_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package github_client - -import ( - "fmt" - "testing" - - "github.com/spf13/viper" - "github.com/stretchr/testify/assert" -) - -func TestCreateClient(t *testing.T) { - viper.Set("vcs-token", "pass") - githubClient := createGithubClient() - assert.Equal(t, "https://api.github.com/", githubClient.BaseURL.String(), fmt.Sprintf("api URL in githubClient (%s) does not match github public API", githubClient.BaseURL.String())) -} diff --git a/pkg/message.go b/pkg/message.go index e047c8f2..08b28f94 100644 --- a/pkg/message.go +++ b/pkg/message.go @@ -94,8 +94,8 @@ func (m *Message) SetFooter(start time.Time, commitSha string) { m.footer = buildFooter(start, commitSha) } -func (m *Message) PushComment(ctx context.Context, client Client) error { - return client.UpdateMessage(ctx, m, buildComment(ctx, m.apps)) +func (m *Message) BuildComment(ctx context.Context) string { + return buildComment(ctx, m.apps) } func buildFooter(start time.Time, commitSHA string) string { diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index 1483d65d..e431eb85 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -48,15 +48,14 @@ func (r *Repo) CloneRepoLocal(ctx context.Context, repoDir string) error { // TODO: Look if this is still needed r.RepoDir = repoDir - cmd := execCommand("git", "clone", r.CloneURL, repoDir) + cmd := r.execCommand("git", "clone", r.CloneURL, repoDir) out, err := cmd.CombinedOutput() if err != nil { log.Error().Err(err).Msgf("unable to clone repository, %s", out) return err } - cmd = execCommand("git", "remote") - cmd.Dir = repoDir + cmd = r.execCommand("git", "remote") pipe, _ := cmd.StdoutPipe() var wg sync.WaitGroup scanner := bufio.NewScanner(pipe) @@ -110,8 +109,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error { defer span.End() log.Debug().Msgf("Merging MR commit %s into a tmp branch off of %s for manifest generation...", r.SHA, r.BaseRef) - cmd := execCommand("git", "fetch", r.Remote, r.BaseRef) - cmd.Dir = r.RepoDir + cmd := r.execCommand("git", "fetch", r.Remote, r.BaseRef) err := cmd.Run() if err != nil { telemetry.SetError(span, err, "git fetch remote into target branch") @@ -119,8 +117,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error { return err } - cmd = execCommand("git", "checkout", "-b", "tmp", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef)) - cmd.Dir = r.RepoDir + cmd = r.execCommand("git", "checkout", "-b", "tmp", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef)) _, err = cmd.Output() if err != nil { telemetry.SetError(span, err, "git checkout tmp branch") @@ -128,8 +125,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error { return err } - cmd = execCommand("git", "merge", r.SHA) - cmd.Dir = r.RepoDir + cmd = r.execCommand("git", "merge", r.SHA) out, err := cmd.CombinedOutput() if err != nil { telemetry.SetError(span, err, "merge last commit id into tmp branch") @@ -172,8 +168,7 @@ func (r *Repo) GetListOfChangedFiles(ctx context.Context) ([]string, error) { var fileList = []string{} - cmd := execCommand("git", "diff", "--name-only", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef)) - cmd.Dir = r.RepoDir + cmd := r.execCommand("git", "diff", "--name-only", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef)) pipe, _ := cmd.StdoutPipe() var wg sync.WaitGroup scanner := bufio.NewScanner(pipe) @@ -210,21 +205,33 @@ func walk(s string, d fs.DirEntry, err error) error { return nil } +func (r *Repo) execCommand(name string, args ...string) *exec.Cmd { + cmd := execCommand(name, args...) + cmd.Dir = r.RepoDir + return cmd +} + +func execCommand(name string, args ...string) *exec.Cmd { + log.Debug().Strs("args", args).Msg("building command") + cmd := exec.Command(name, args...) + return cmd +} + // InitializeGitSettings ensures Git auth is set up for cloning -func InitializeGitSettings(user string, email string) error { +func InitializeGitSettings(username, email string) error { cmd := execCommand("git", "config", "--global", "user.email", email) err := cmd.Run() if err != nil { return errors.Wrap(err, "failed to set git email address") } - cmd = execCommand("git", "config", "--global", "user.name", user) + cmd = execCommand("git", "config", "--global", "user.name", username) err = cmd.Run() if err != nil { return errors.Wrap(err, "failed to set git user name") } - cloneUrl, err := getCloneUrl(user, viper.GetViper()) + cloneUrl, err := getCloneUrl(username, viper.GetViper()) if err != nil { return errors.Wrap(err, "failed to get clone url") } @@ -279,8 +286,3 @@ func getCloneUrl(user string, cfg *viper.Viper) (string, error) { } return fmt.Sprintf("%s://%s:%s@%s", scheme, user, vcsToken, hostname), nil } - -func execCommand(name string, args ...string) *exec.Cmd { - log.Debug().Strs("args", args).Msg("building command") - return exec.Command(name, args...) -} diff --git a/pkg/server/hook_handler.go b/pkg/server/hook_handler.go index 06c3bb14..67fa6614 100644 --- a/pkg/server/hook_handler.go +++ b/pkg/server/hook_handler.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "strings" - "sync" "github.com/labstack/echo/v4" "github.com/rs/zerolog/log" @@ -14,68 +13,32 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" - "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/config" "github.com/zapier/kubechecks/pkg/events" - "github.com/zapier/kubechecks/pkg/github_client" - "github.com/zapier/kubechecks/pkg/gitlab_client" "github.com/zapier/kubechecks/pkg/repo" + "github.com/zapier/kubechecks/pkg/vcs" "github.com/zapier/kubechecks/telemetry" ) type VCSHookHandler struct { - client pkg.Client - tokenUser string - cfg *config.ServerConfig + client vcs.Client + cfg *config.ServerConfig // labelFilter is a string specifying the required label name to filter merge events by; if empty, all merge events will pass the filter. labelFilter string } -var once sync.Once -var vcsClient pkg.Client // Currently, only allow one client at a time -var tokenUser string -var ProjectHookPath = "/gitlab/project" - -// High level type representing the fields we care about from an arbitrary Git repository -func GetVCSClient() (pkg.Client, string) { - once.Do(func() { - vcsClient, tokenUser = createVCSClient() - }) - return vcsClient, tokenUser -} - -func createVCSClient() (pkg.Client, string) { - // Determine what client to use based on set config (default Gitlab) - clientType := viper.GetString("vcs-type") - // All hooks set up follow the convention /VCS_PROVIDER/project - ProjectHookPath = fmt.Sprintf("/%s/project", clientType) - switch clientType { - case "gitlab": - return gitlab_client.GetGitlabClient() - case "github": - return github_client.GetGithubClient() - default: - log.Fatal().Msgf("Unknown VCS type: %s", clientType) - return nil, "" - } - -} - func NewVCSHookHandler(cfg *config.ServerConfig) *VCSHookHandler { - client, tokenUser := GetVCSClient() labelFilter := viper.GetString("label-filter") return &VCSHookHandler{ - client: client, - tokenUser: tokenUser, + client: cfg.VcsClient, cfg: cfg, labelFilter: labelFilter, } } func (h *VCSHookHandler) AttachHandlers(grp *echo.Group) { - log.Info().Str("path", GetServer().hooksPrefix()).Msg("setting up hook handler") - grp.POST(ProjectHookPath, h.groupHandler) - log.Info().Str("path", GetServer().hooksPrefix()).Str("projectPath", ProjectHookPath).Msg("hook handler setup complete") + projectHookPath := fmt.Sprintf("/%s/project", h.cfg.VcsClient.GetName()) + grp.POST(projectHookPath, h.groupHandler) } func (h *VCSHookHandler) groupHandler(c echo.Context) error { @@ -88,19 +51,10 @@ func (h *VCSHookHandler) groupHandler(c echo.Context) error { return c.String(http.StatusUnauthorized, "Unauthorized") } - eventRequest, err := h.client.ParseHook(c.Request(), payload) - if err != nil { - // TODO: do something w/ error - log.Error().Err(err).Msg("Failed to parse hook payload. Are you using the right client?") - return echo.ErrBadRequest - } - - // At this point, our client has validated the secret, and parsed a valid event. - // We try to build a generic Repo from this data, to construct our CheckEvent - repo, err := h.client.CreateRepo(ctx, eventRequest) + r, err := h.client.ParseHook(c.Request(), payload) if err != nil { switch err { - case pkg.ErrInvalidType: + case vcs.ErrInvalidType: log.Debug().Msg("Ignoring event, not a merge request") return c.String(http.StatusOK, "Skipped") default: @@ -111,33 +65,37 @@ func (h *VCSHookHandler) groupHandler(c echo.Context) error { } // We now have a generic repo with all the info we need to start processing an event. Hand off to the event processor - go h.processCheckEvent(ctx, repo) + go h.processCheckEvent(ctx, r) return c.String(http.StatusAccepted, "Accepted") } // Takes a constructed Repo, and attempts to run the Kubechecks processing suite against it. // If the Repo is not yet populated, this will fail. func (h *VCSHookHandler) processCheckEvent(ctx context.Context, repo *repo.Repo) { + if !h.passesLabelFilter(repo) { + log.Warn().Str("label-filter", h.labelFilter).Msg("ignoring event, did not have matching label") + return + } + + ProcessCheckEvent(ctx, repo, h.cfg) +} + +func ProcessCheckEvent(ctx context.Context, r *repo.Repo, cfg *config.ServerConfig) { var span trace.Span ctx, span = otel.Tracer("Kubechecks").Start(ctx, "processCheckEvent", trace.WithAttributes( - attribute.Int("mr_id", repo.CheckID), - attribute.String("project", repo.Name), - attribute.String("sha", repo.SHA), - attribute.String("source", repo.HeadRef), - attribute.String("target", repo.BaseRef), - attribute.String("default_branch", repo.DefaultBranch), + attribute.Int("mr_id", r.CheckID), + attribute.String("project", r.Name), + attribute.String("sha", r.SHA), + attribute.String("source", r.HeadRef), + attribute.String("target", r.BaseRef), + attribute.String("default_branch", r.DefaultBranch), ), ) defer span.End() - if !h.passesLabelFilter(repo) { - log.Warn().Str("label-filter", h.labelFilter).Msg("ignoring event, did not have matching label") - return - } - // If we've gotten here, we can now begin running checks (or trying to) - cEvent := events.NewCheckEvent(repo, h.client, h.cfg) + cEvent := events.NewCheckEvent(r, cfg) err := cEvent.CreateTempDir() if err != nil { @@ -146,12 +104,13 @@ func (h *VCSHookHandler) processCheckEvent(ctx context.Context, repo *repo.Repo) } defer cEvent.Cleanup(ctx) - err = cEvent.InitializeGit(ctx) + err = repo.InitializeGitSettings(cfg.VcsClient.Username(), cfg.VcsClient.Email()) if err != nil { telemetry.SetError(span, err, "Initialize Git") log.Error().Err(err).Msg("unable to initialize git") return } + // Clone the repo's BaseRef (main etc) locally into the temp dir we just made err = cEvent.CloneRepoLocal(ctx) if err != nil { @@ -178,7 +137,7 @@ func (h *VCSHookHandler) processCheckEvent(ctx context.Context, repo *repo.Repo) } // Generate a list of affected apps, storing them within the CheckEvent (also returns but discarded here) - err = cEvent.GenerateListOfAffectedApps(ctx, repo.BaseRef) + err = cEvent.GenerateListOfAffectedApps(ctx, r.BaseRef) if err != nil { // TODO: Cancel if gitlab etc //mEvent.CancelEvent(ctx, err, "Generate List of Affected Apps") diff --git a/pkg/server/server.go b/pkg/server/server.go index f75553d4..fd2abe11 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2,6 +2,7 @@ package server import ( "context" + "fmt" "net/url" "strings" @@ -14,31 +15,25 @@ import ( "github.com/spf13/viper" "github.com/ziflex/lecho/v3" - "github.com/zapier/kubechecks/pkg" - "github.com/zapier/kubechecks/pkg/argo_client" "github.com/zapier/kubechecks/pkg/config" + "github.com/zapier/kubechecks/pkg/vcs" ) const KubeChecksHooksPathPrefix = "/hooks" -var singleton *Server - type Server struct { cfg *config.ServerConfig } func NewServer(cfg *config.ServerConfig) *Server { - singleton = &Server{cfg: cfg} - return singleton -} - -func GetServer() *Server { - return singleton + return &Server{cfg: cfg} } -func (s *Server) Start() { - if err := s.buildVcsToArgoMap(); err != nil { +func (s *Server) Start(ctx context.Context) { + if argoMap, err := s.buildVcsToArgoMap(ctx); err != nil { log.Warn().Err(err).Msg("failed to build vcs app map from argo") + } else { + s.cfg.VcsToArgoMap = argoMap } if err := s.ensureWebhooks(); err != nil { @@ -63,6 +58,11 @@ func (s *Server) Start() { ghHooks := NewVCSHookHandler(s.cfg) ghHooks.AttachHandlers(hooksGroup) + fmt.Println("Method\tPath") + for _, r := range e.Routes() { + fmt.Printf("%s\t%s\n", r.Method, r.Path) + } + if err := e.Start(":8080"); err != nil { log.Fatal().Err(err).Msg("could not start hooks server") } @@ -95,7 +95,7 @@ func (s *Server) ensureWebhooks() error { log.Info().Msg("ensuring all webhooks are created correctly") ctx := context.TODO() - vcsClient, _ := GetVCSClient() + vcsClient := s.cfg.VcsClient fullUrl, err := url.JoinPath(urlBase, s.hooksPrefix(), vcsClient.GetName(), "project") if err != nil { @@ -106,7 +106,7 @@ func (s *Server) ensureWebhooks() error { for _, repo := range s.cfg.GetVcsRepos() { wh, err := vcsClient.GetHookByUrl(ctx, repo, fullUrl) - if err != nil && !errors.Is(err, pkg.ErrHookNotFound) { + if err != nil && !errors.Is(err, vcs.ErrHookNotFound) { log.Error().Err(err).Msgf("failed to get hook for %s:", repo) continue } @@ -121,26 +121,12 @@ func (s *Server) ensureWebhooks() error { return nil } -func (s *Server) buildVcsToArgoMap() error { - log.Debug().Msg("building VCS to Application Map") +func (s *Server) buildVcsToArgoMap(ctx context.Context) (config.VcsToArgoMap, error) { if !viper.GetBool("monitor-all-applications") { - return nil + return config.NewVcsToArgoMap(), nil } - ctx := context.TODO() - - result := config.NewVcsToArgoMap() - - argoClient := argo_client.GetArgoClient() - - apps, err := argoClient.GetApplications(ctx) - if err != nil { - return errors.Wrap(err, "failed to list applications") - } - for _, app := range apps.Items { - result.AddApp(app) - } + log.Debug().Msg("building VCS to Application Map") - s.cfg.VcsToArgoMap = result - return nil + return config.BuildAppsMap(ctx) } diff --git a/pkg/client.go b/pkg/vcs/client.go similarity index 70% rename from pkg/client.go rename to pkg/vcs/client.go index 76311e8f..b5881fa1 100644 --- a/pkg/client.go +++ b/pkg/vcs/client.go @@ -1,10 +1,11 @@ -package pkg +package vcs import ( "context" "errors" "net/http" + "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/repo" ) @@ -23,23 +24,26 @@ type WebHookConfig struct { // Client represents a VCS client type Client interface { // PostMessage takes in project name in form "owner/repo" (ie zapier/kubechecks), the PR/MR id, and the actual message - PostMessage(context.Context, *repo.Repo, int, string) *Message + PostMessage(context.Context, *repo.Repo, int, string) *pkg.Message // UpdateMessage update a message with new content - UpdateMessage(context.Context, *Message, string) error + UpdateMessage(context.Context, *pkg.Message, string) error // VerifyHook validates a webhook secret and return the body; must be called even if no secret VerifyHook(*http.Request, string) ([]byte, error) // ParseHook parses webook payload for valid events - ParseHook(*http.Request, []byte) (interface{}, error) - // CreateRepo handles valid events - CreateRepo(context.Context, interface{}) (*repo.Repo, error) + ParseHook(*http.Request, []byte) (*repo.Repo, error) // CommitStatus sets a status for a specific commit on the remote VCS - CommitStatus(context.Context, *repo.Repo, CommitState) error + CommitStatus(context.Context, *repo.Repo, pkg.CommitState) error // GetHookByUrl gets a webhook by url GetHookByUrl(ctx context.Context, repoName, webhookUrl string) (*WebHookConfig, error) // CreateHook creates a webhook that points at kubechecks CreateHook(ctx context.Context, repoName, webhookUrl, webhookSecret string) error // GetName returns the VCS client name (e.g. "github" or "gitlab") GetName() string - // Tidy outdated comments either by hiding or deleting them + // TidyOutdatedComments either by hiding or deleting them TidyOutdatedComments(context.Context, *repo.Repo) error + // LoadHook creates an EventRequest from the ID of an actual request + LoadHook(ctx context.Context, repoAndId string) (*repo.Repo, error) + + Username() string + Email() string } diff --git a/pkg/github_client/client.go b/pkg/vcs/github_client/client.go similarity index 63% rename from pkg/github_client/client.go rename to pkg/vcs/github_client/client.go index c0c130ef..09b14f99 100644 --- a/pkg/github_client/client.go +++ b/pkg/vcs/github_client/client.go @@ -4,8 +4,9 @@ import ( "context" "io" "net/http" + "regexp" + "strconv" "strings" - "sync" "github.com/google/go-github/v53/github" "github.com/pkg/errors" @@ -16,41 +17,21 @@ import ( "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/repo" + "github.com/zapier/kubechecks/pkg/vcs" ) -var githubClient *Client -var githubTokenUser string -var once sync.Once // used to ensure we don't reauth this - type Client struct { - v4Client *githubv4.Client + v4Client *githubv4.Client + username, email string + *github.Client } -var _ pkg.Client = new(Client) - -func GetGithubClient() (*Client, string) { - once.Do(func() { - githubClient = createGithubClient() - githubTokenUser = getTokenUser() - }) - return githubClient, githubTokenUser -} +var _ vcs.Client = new(Client) -// We require a username to use with git locally, so get the current auth'd user -func getTokenUser() string { - user, _, err := githubClient.Users.Get(context.Background(), "") - if err != nil { - if err != nil { - log.Fatal().Err(err).Msg("could not get Github user") - } - } - return *user.Login -} - -// Create a new GitHub client using the auth token provided. We +// CreateGithubClient creates a new GitHub client using the auth token provided. We // can't validate the token at this point, so if it exists we assume it works -func createGithubClient() *Client { +func CreateGithubClient() (*Client, error) { var ( err error googleClient *github.Client @@ -83,9 +64,21 @@ func createGithubClient() *Client { shurcoolClient = githubv4.NewEnterpriseClient(githubUrl, tc) } - return &Client{Client: googleClient, v4Client: shurcoolClient} + user, _, err := googleClient.Users.Get(ctx, "") + if err != nil { + return nil, errors.Wrap(err, "failed to get user") + } + + return &Client{ + Client: googleClient, + v4Client: shurcoolClient, + username: *user.Login, + email: *user.Email, + }, nil } +func (c *Client) Username() string { return c.username } +func (c *Client) Email() string { return c.email } func (c *Client) GetName() string { return "github" } @@ -101,55 +94,29 @@ func (c *Client) VerifyHook(r *http.Request, secret string) ([]byte, error) { } } -func (c *Client) ParseHook(r *http.Request, payload []byte) (interface{}, error) { - return github.ParseWebHook(github.WebHookType(r), payload) -} +func (c *Client) ParseHook(r *http.Request, request []byte) (*repo.Repo, error) { + payload, err := github.ParseWebHook(github.WebHookType(r), request) + if err != nil { + return nil, err + } -// CreateRepo creates a new generic repo from the webhook payload. Assumes the secret validation/type validation -// Has already occured previously, so we expect a valid event type for the GitHub client in the payload arg -func (c *Client) CreateRepo(_ context.Context, payload interface{}) (*repo.Repo, error) { switch p := payload.(type) { case *github.PullRequestEvent: switch p.GetAction() { case "opened", "synchronize", "reopened", "edited": log.Info().Str("action", p.GetAction()).Msg("handling Github event from PR") - return buildRepoFromEvent(p), nil + return c.buildRepoFromEvent(p), nil default: log.Info().Str("action", p.GetAction()).Msg("ignoring Github pull request event due to non commit based action") - return nil, pkg.ErrInvalidType + return nil, vcs.ErrInvalidType } default: log.Error().Msg("invalid event provided to Github client") - return nil, pkg.ErrInvalidType + return nil, vcs.ErrInvalidType } } -// We need an email and username for authenticating our local git repository -// Grab the current authenticated client login and email -func (c *Client) getUserDetails() (string, string, error) { - user, _, err := c.Users.Get(context.Background(), "") - if err != nil { - return "", "", err - } - - // Some users on GitHub don't have an email listed; if so, catch that and return empty string - if user.Email == nil { - log.Error().Msg("could not load Github user email") - return *user.Login, "", nil - } - - return *user.Login, *user.Email, nil - -} - -func buildRepoFromEvent(event *github.PullRequestEvent) *repo.Repo { - username, email, err := githubClient.getUserDetails() - if err != nil { - log.Fatal().Err(err).Msg("could not load Github user details") - username = "" - email = "" - } - +func (c *Client) buildRepoFromEvent(event *github.PullRequestEvent) *repo.Repo { var labels []string for _, label := range event.PullRequest.Labels { labels = append(labels, label.GetName()) @@ -165,8 +132,8 @@ func buildRepoFromEvent(event *github.PullRequestEvent) *repo.Repo { Name: event.Repo.GetName(), CheckID: *event.PullRequest.Number, SHA: *event.PullRequest.Head.SHA, - Username: username, - Email: email, + Username: c.username, + Email: c.email, Labels: labels, } } @@ -217,7 +184,7 @@ func parseRepo(cloneUrl string) (string, string) { panic(cloneUrl) } -func (c *Client) GetHookByUrl(ctx context.Context, ownerAndRepoName, webhookUrl string) (*pkg.WebHookConfig, error) { +func (c *Client) GetHookByUrl(ctx context.Context, ownerAndRepoName, webhookUrl string) (*vcs.WebHookConfig, error) { owner, repoName := parseRepo(ownerAndRepoName) items, _, err := c.Repositories.ListHooks(ctx, owner, repoName, nil) if err != nil { @@ -226,14 +193,14 @@ func (c *Client) GetHookByUrl(ctx context.Context, ownerAndRepoName, webhookUrl for _, item := range items { if item.URL != nil && *item.URL == webhookUrl { - return &pkg.WebHookConfig{ + return &vcs.WebHookConfig{ Url: item.GetURL(), Events: item.Events, // TODO: translate GH specific event names to VCS agnostic }, nil } } - return nil, pkg.ErrHookNotFound + return nil, vcs.ErrHookNotFound } func (c *Client) CreateHook(ctx context.Context, ownerAndRepoName, webhookUrl, webhookSecret string) error { @@ -257,3 +224,91 @@ func (c *Client) CreateHook(ctx context.Context, ownerAndRepoName, webhookUrl, w return nil } + +var rePullRequest = regexp.MustCompile(`(.*)/(.*)#(\d+)`) + +func (c *Client) LoadHook(ctx context.Context, id string) (*repo.Repo, error) { + m := rePullRequest.FindStringSubmatch(id) + if len(m) != 4 { + return nil, errors.New("must be in format OWNER/REPO#PR") + } + + ownerName := m[1] + repoName := m[2] + prNumber, err := strconv.ParseInt(m[3], 10, 32) + if err != nil { + return nil, errors.Wrap(err, "failed to parse int") + } + + repoInfo, _, err := c.Repositories.Get(ctx, ownerName, repoName) + if err != nil { + return nil, errors.Wrap(err, "failed to get repo") + } + + pullRequest, _, err := c.PullRequests.Get(ctx, ownerName, repoName, int(prNumber)) + if err != nil { + return nil, errors.Wrap(err, "failed to get pull request") + } + + var labels []string + for _, label := range pullRequest.Labels { + labels = append(labels, label.GetName()) + } + + var ( + baseRef string + headRef, headSha string + login, userName, userEmail string + ) + + if pullRequest.Base != nil { + baseRef = unPtr(pullRequest.Base.Ref) + headRef = unPtr(pullRequest.Head.Ref) + } + + if repoInfo.Owner != nil { + login = unPtr(repoInfo.Owner.Login) + } else { + login = "kubechecks" + } + + if pullRequest.Head != nil { + headSha = unPtr(pullRequest.Head.SHA) + } + + if pullRequest.User != nil { + userName = unPtr(pullRequest.User.Name) + userEmail = unPtr(pullRequest.User.Email) + } + + // these are required for `git merge` later on + if userName == "" { + userName = "kubechecks" + } + if userEmail == "" { + userEmail = "kubechecks@github.com" + } + + return &repo.Repo{ + BaseRef: baseRef, + HeadRef: headRef, + DefaultBranch: unPtr(repoInfo.DefaultBranch), + CloneURL: unPtr(repoInfo.CloneURL), + FullName: repoInfo.GetFullName(), + Owner: login, + Name: repoInfo.GetName(), + CheckID: int(prNumber), + SHA: headSha, + Username: userName, + Email: userEmail, + Labels: labels, + }, nil +} + +func unPtr[T interface{ string | int }](ps *T) T { + if ps == nil { + var t T + return t + } + return *ps +} diff --git a/pkg/github_client/message.go b/pkg/vcs/github_client/message.go similarity index 97% rename from pkg/github_client/message.go rename to pkg/vcs/github_client/message.go index 5f25346c..af2366d4 100644 --- a/pkg/github_client/message.go +++ b/pkg/vcs/github_client/message.go @@ -86,7 +86,7 @@ func (c *Client) pruneOldComments(ctx context.Context, repo *repo.Repo, comments log.Debug().Msgf("Pruning messages from PR %d in repo %s", repo.CheckID, repo.FullName) for _, comment := range comments { - if strings.EqualFold(comment.GetUser().GetLogin(), githubTokenUser) { + if strings.EqualFold(comment.GetUser().GetLogin(), c.username) { _, err := c.Issues.DeleteComment(ctx, repo.Owner, repo.Name, *comment.ID) if err != nil { return fmt.Errorf("failed to delete comment: %w", err) @@ -104,7 +104,7 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, repo *repo.Repo, comm log.Debug().Msgf("Hiding kubecheck messages in PR %d in repo %s", repo.CheckID, repo.FullName) for _, comment := range comments { - if strings.EqualFold(comment.GetUser().GetLogin(), githubTokenUser) { + if strings.EqualFold(comment.GetUser().GetLogin(), c.username) { // Github API does not expose minimizeComment API. IT's only available from the GraphQL API // https://docs.github.com/en/graphql/reference/mutations#minimizecomment var m struct { diff --git a/pkg/gitlab_client/backoff.go b/pkg/vcs/gitlab_client/backoff.go similarity index 100% rename from pkg/gitlab_client/backoff.go rename to pkg/vcs/gitlab_client/backoff.go diff --git a/pkg/gitlab_client/client.go b/pkg/vcs/gitlab_client/client.go similarity index 64% rename from pkg/gitlab_client/client.go rename to pkg/vcs/gitlab_client/client.go index 4ebca3cb..877562ab 100644 --- a/pkg/gitlab_client/client.go +++ b/pkg/vcs/gitlab_client/client.go @@ -5,8 +5,9 @@ import ( "fmt" "io" "net/http" + "regexp" + "strconv" "strings" - "sync" "github.com/pkg/errors" "github.com/rs/zerolog/log" @@ -16,31 +17,20 @@ import ( "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/repo" + "github.com/zapier/kubechecks/pkg/vcs" ) -var gitlabClient *Client -var gitlabTokenUser string -var gitlabTokenEmail string -var once sync.Once - const GitlabTokenHeader = "X-Gitlab-Token" type Client struct { *gitlab.Client -} -var _ pkg.Client = new(Client) - -func GetGitlabClient() (*Client, string) { - once.Do(func() { - gitlabClient = createGitlabClient() - gitlabTokenUser, gitlabTokenEmail = gitlabClient.getTokenUser() - }) - - return gitlabClient, gitlabTokenUser + username, email string } -func createGitlabClient() *Client { +var _ vcs.Client = new(Client) + +func CreateGitlabClient() (*Client, error) { // Initialize the GitLab client with access token gitlabToken := viper.GetString("vcs-token") if gitlabToken == "" { @@ -60,18 +50,16 @@ func createGitlabClient() *Client { log.Fatal().Err(err).Msg("could not create Gitlab client") } - return &Client{c} -} - -func (c *Client) getTokenUser() (string, string) { user, _, err := c.Users.CurrentUser() if err != nil { - log.Fatal().Err(err).Msg("could not create Gitlab token user") + return nil, errors.Wrap(err, "failed to get current user") } - return user.Username, user.Email + return &Client{Client: c, username: user.Username, email: user.Email}, nil } +func (c *Client) Email() string { return c.email } +func (c *Client) Username() string { return c.username } func (c *Client) GetName() string { return "gitlab" } @@ -89,32 +77,31 @@ func (c *Client) VerifyHook(r *http.Request, secret string) ([]byte, error) { } // ParseHook parses and validates a webhook event; return an err if this isn't valid -func (c *Client) ParseHook(r *http.Request, payload []byte) (interface{}, error) { - return gitlab.ParseHook(gitlab.HookEventType(r), payload) -} +func (c *Client) ParseHook(r *http.Request, request []byte) (*repo.Repo, error) { + eventRequest, err := gitlab.ParseHook(gitlab.HookEventType(r), request) + if err != nil { + return nil, err + } -// CreateRepo takes a valid gitlab webhook event request, and determines if we should process it -// Returns a generic Repo with all info kubechecks needs on success, err if not -func (c *Client) CreateRepo(ctx context.Context, eventRequest interface{}) (*repo.Repo, error) { switch event := eventRequest.(type) { case *gitlab.MergeEvent: switch event.ObjectAttributes.Action { case "update": if event.ObjectAttributes.OldRev != "" && event.ObjectAttributes.OldRev != event.ObjectAttributes.LastCommit.ID { - return buildRepoFromEvent(event), nil + return c.buildRepoFromEvent(event), nil } log.Trace().Msgf("Skipping update event sha didn't change") case "open", "reopen": - return buildRepoFromEvent(event), nil + return c.buildRepoFromEvent(event), nil default: log.Trace().Msgf("Unhandled Action %s", event.ObjectAttributes.Action) - return nil, pkg.ErrInvalidType + return nil, vcs.ErrInvalidType } default: log.Trace().Msgf("Unhandled Event: %T", event) - return nil, pkg.ErrInvalidType + return nil, vcs.ErrInvalidType } - return nil, pkg.ErrInvalidType + return nil, vcs.ErrInvalidType } func parseRepoName(url string) (string, error) { @@ -129,7 +116,7 @@ func parseRepoName(url string) (string, error) { return path, nil } -func (c *Client) GetHookByUrl(ctx context.Context, repoName, webhookUrl string) (*pkg.WebHookConfig, error) { +func (c *Client) GetHookByUrl(ctx context.Context, repoName, webhookUrl string) (*vcs.WebHookConfig, error) { pid, err := parseRepoName(repoName) if err != nil { return nil, errors.Wrap(err, "failed to parse repo url") @@ -146,14 +133,14 @@ func (c *Client) GetHookByUrl(ctx context.Context, repoName, webhookUrl string) if hook.MergeRequestsEvents { events = append(events, string(gitlab.MergeRequestEventTargetType)) } - return &pkg.WebHookConfig{ + return &vcs.WebHookConfig{ Url: hook.URL, Events: events, }, nil } } - return nil, pkg.ErrHookNotFound + return nil, vcs.ErrHookNotFound } func (c *Client) CreateHook(ctx context.Context, repoName, webhookUrl, webhookSecret string) error { @@ -175,7 +162,49 @@ func (c *Client) CreateHook(ctx context.Context, repoName, webhookUrl, webhookSe return nil } -func buildRepoFromEvent(event *gitlab.MergeEvent) *repo.Repo { +var reMergeRequest = regexp.MustCompile(`(.*)!(\d+)`) + +func (c *Client) LoadHook(ctx context.Context, id string) (*repo.Repo, error) { + m := reMergeRequest.FindStringSubmatch(id) + if len(m) != 3 { + return nil, errors.New("must be in format REPOPATH!MR") + } + + repoPath := m[1] + mrNumber, err := strconv.ParseInt(m[2], 10, 32) + if err != nil { + return nil, errors.Wrap(err, "failed to parse merge request number") + } + + project, _, err := c.Projects.GetProject(repoPath, nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to get project '%s'", repoPath) + } + + mergeRequest, _, err := c.MergeRequests.GetMergeRequest(repoPath, int(mrNumber), nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to get merge request '%d' in project '%s'", mrNumber, repoPath) + } + + return &repo.Repo{ + BaseRef: mergeRequest.TargetBranch, + HeadRef: mergeRequest.SourceBranch, + DefaultBranch: project.DefaultBranch, + RepoDir: "", + Remote: "", + CloneURL: project.HTTPURLToRepo, + Name: project.Name, + Owner: "", + CheckID: mergeRequest.IID, + SHA: mergeRequest.SHA, + FullName: project.PathWithNamespace, + Username: c.username, + Email: c.email, + Labels: mergeRequest.Labels, + }, nil +} + +func (c *Client) buildRepoFromEvent(event *gitlab.MergeEvent) *repo.Repo { // Convert all labels from this MR to a string array of label names var labels []string for _, label := range event.Labels { @@ -191,8 +220,8 @@ func buildRepoFromEvent(event *gitlab.MergeEvent) *repo.Repo { Name: event.Project.Name, CheckID: event.ObjectAttributes.IID, SHA: event.ObjectAttributes.LastCommit.ID, - Username: gitlabTokenUser, - Email: gitlabTokenEmail, + Username: c.username, + Email: c.email, Labels: labels, } } diff --git a/pkg/gitlab_client/client_test.go b/pkg/vcs/gitlab_client/client_test.go similarity index 72% rename from pkg/gitlab_client/client_test.go rename to pkg/vcs/gitlab_client/client_test.go index f7013c48..c86c60ce 100644 --- a/pkg/gitlab_client/client_test.go +++ b/pkg/vcs/gitlab_client/client_test.go @@ -1,20 +1,12 @@ package gitlab_client import ( - "fmt" "testing" - "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestCreateClient(t *testing.T) { - viper.Set("vcs-token", "pass") - gitlabClient := createGitlabClient() - assert.Equal(t, "https://gitlab.com/api/v4/", gitlabClient.BaseURL().String(), fmt.Sprintf("api URL in githubClient (%s) does not match github public API", gitlabClient.BaseURL().String())) -} - func TestCustomGitURLParsing(t *testing.T) { testcases := []struct { giturl, expected string diff --git a/pkg/gitlab_client/merge.go b/pkg/vcs/gitlab_client/merge.go similarity index 100% rename from pkg/gitlab_client/merge.go rename to pkg/vcs/gitlab_client/merge.go diff --git a/pkg/gitlab_client/message.go b/pkg/vcs/gitlab_client/message.go similarity index 96% rename from pkg/gitlab_client/message.go rename to pkg/vcs/gitlab_client/message.go index 3519d1e4..3e5a2e14 100644 --- a/pkg/gitlab_client/message.go +++ b/pkg/vcs/gitlab_client/message.go @@ -52,7 +52,7 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, projectName string, m // note user is not the gitlabTokenUser // note is an internal system note such as notes on commit messages // note is already hidden - if note.Author.Username != gitlabTokenUser || note.System || strings.Contains(note.Body, "OUTDATED: Kubechecks Report") { + if note.Author.Username != c.username || note.System || strings.Contains(note.Body, "OUTDATED: Kubechecks Report") { continue } @@ -114,7 +114,7 @@ func (c *Client) pruneOldComments(ctx context.Context, projectName string, mrID log.Debug().Msg("deleting outdated comments") for _, note := range notes { - if note.Author.Username == gitlabTokenUser { + if note.Author.Username == c.username { log.Debug().Int("mr", mrID).Int("note", note.ID).Msg("deleting old comment") _, err := c.Notes.DeleteMergeRequestNote(projectName, mrID, note.ID) if err != nil { diff --git a/pkg/gitlab_client/pipeline.go b/pkg/vcs/gitlab_client/pipeline.go similarity index 100% rename from pkg/gitlab_client/pipeline.go rename to pkg/vcs/gitlab_client/pipeline.go diff --git a/pkg/gitlab_client/project.go b/pkg/vcs/gitlab_client/project.go similarity index 100% rename from pkg/gitlab_client/project.go rename to pkg/vcs/gitlab_client/project.go diff --git a/pkg/gitlab_client/status.go b/pkg/vcs/gitlab_client/status.go similarity index 100% rename from pkg/gitlab_client/status.go rename to pkg/vcs/gitlab_client/status.go