-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
handle multi source applications #298
Conversation
Temporary image deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Not sure if it works yet, but it definitely broke kubeconform schema checking. |
bc8b757
to
5e8ca6b
Compare
appreciate the work on this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions. As for the unittest, you can generate easily them all using the genie integration on Vscode with OpenAI or with copilot, if you use either of them.
return ctr, nil | ||
} | ||
|
||
func buildAppsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result appdir.VcsToArgoMap) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unittest
return nil | ||
} | ||
|
||
func buildAppSetsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result appdir.VcsToArgoMap) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unittest
@@ -227,20 +223,36 @@ func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneU | |||
return repo, nil | |||
} | |||
|
|||
func (ce *CheckEvent) mergeIntoTarget(ctx context.Context, repo *git.Repo, branch string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unittest?
# Conflicts: # .tool-versions
Thanks for the review! This gives me a good idea as to what needs to be better documented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes are looking good. I've also tested them on my end and it works fine, Thanks!
The only thing is I think we need to add more tests. We're doing a lot of intricate file/app modifications. We need to catch when it breaks early. I've highlighted some funcs that we can add tests to.
Great work!
Mergecat's ReviewClick to read mergecats review!😼 Mergecat review of charts/kubechecks/Chart.yaml@@ -1,7 +1,7 @@
apiVersion: v2
name: kubechecks
description: A Helm chart for kubechecks
-version: 0.4.6
+version: 0.5.0
type: application
maintainers:
- name: zapier Feedback & Suggestions:
😼 Mergecat review of .tool-versions@@ -6,5 +6,6 @@ helm-cr 1.6.1
helm-ct 3.11.0
kubeconform 0.6.7
kustomize 5.5.0
+mockery 2.46.3
staticcheck 2024.1.1
tilt 0.33.2 Feedback & Suggestions:
😼 Mergecat review of cmd/locations.go@@ -25,6 +25,8 @@ func processLocations(ctx context.Context, ctr container.Container, locations []
}
}
+ log.Debug().Strs("locations", locations).Msg("locations after processing")
+
return nil
}
Feedback & Suggestions:
😼 Mergecat review of pkg/checks/kubeconform/check.go@@ -8,8 +8,5 @@ import (
)
func Check(ctx context.Context, request checks.Request) (msg.Result, error) {
- return argoCdAppValidate(
- ctx, request.Container, request.AppName, request.KubernetesVersion, request.Repo.Directory,
- request.YamlManifests,
- )
+ return argoCdAppValidate(ctx, request.Container, request.AppName, request.KubernetesVersion, request.YamlManifests)
} Feedback & Suggestions:
😼 Mergecat review of charts/kubechecks/values.yaml@@ -1,4 +1,7 @@
# Labels to apply to all resources created by this Helm chart
+argocd:
+ namespace: argocd
+
commonLabels: {}
configMap: Feedback & Suggestions:
😼 Mergecat review of mocks/gitlab_client/mocks/mock_RepositoryFilesServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockRepositoryFilesServices) GetRawFile(pid interface{}, fileName stri
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for GetRawFile")
+ }
+
var r0 []byte
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of .mockery.yaml@@ -1,6 +1,9 @@
with-expecter: true
dir: "mocks/{{.PackageName}}/mocks"
packages:
+ github.com/zapier/kubechecks/pkg/vcs:
+ config:
+ all: true
github.com/zapier/kubechecks/pkg/vcs/github_client:
# place your package-specific config here
config: Feedback & Suggestions:
😼 Mergecat review of mocks/gitlab_client/mocks/mock_CommitsServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockCommitsServices) SetCommitStatus(pid interface{}, sha string, opt
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for SetCommitStatus")
+ }
+
var r0 *gitlab.CommitStatus
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of mocks/gitlab_client/mocks/mock_PipelinesServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockPipelinesServices) ListProjectPipelines(pid interface{}, opt *gitl
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for ListProjectPipelines")
+ }
+
var r0 []*gitlab.PipelineInfo
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of mocks/affected_apps/mocks/mock_Matcher.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package affected_apps
@@ -29,6 +29,10 @@ func (_m *MockMatcher) EXPECT() *MockMatcher_Expecter {
func (_m *MockMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string, repo *git.Repo) (affected_apps.AffectedItems, error) {
ret := _m.Called(ctx, changeList, targetBranch, repo)
+ if len(ret) == 0 {
+ panic("no return value specified for AffectedApps")
+ }
+
var r0 affected_apps.AffectedItems
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, []string, string, *git.Repo) (affected_apps.AffectedItems, error)); ok { Feedback & Suggestions:
😼 Mergecat review of mocks/generator/mocks/mock_AppsGenerator.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package generator
@@ -29,6 +29,10 @@ func (_m *MockAppsGenerator) EXPECT() *MockAppsGenerator_Expecter {
func (_m *MockAppsGenerator) GenerateApplicationSetApps(ctx context.Context, appset v1alpha1.ApplicationSet, ctr *container.Container) ([]v1alpha1.Application, error) {
ret := _m.Called(ctx, appset, ctr)
+ if len(ret) == 0 {
+ panic("no return value specified for GenerateApplicationSetApps")
+ }
+
var r0 []v1alpha1.Application
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, v1alpha1.ApplicationSet, *container.Container) ([]v1alpha1.Application, error)); ok { Feedback & Suggestions:
😼 Mergecat review of pkg/repoUrl_test.go+
+func TestAreSameRepos(t *testing.T) {
+ testcases := map[string]struct {
+ input1, input2 string
+ expected bool
+ }{
+ "empty": {"", "", true},
+ "empty1": {"", "blah", false},
+ "empty2": {"blah", "", false},
+ "git-to-git": {"[email protected]:zapier/kubechecks.git", "[email protected]:zapier/kubechecks.git", true},
+ "no-git-suffix-to-git": {"[email protected]:zapier/kubechecks", "[email protected]:zapier/kubechecks.git", true},
+ "https-to-git": {"https://github.com/zapier/kubechecks", "[email protected]:zapier/kubechecks.git", true},
+ }
+ for name, tc := range testcases {
+ t.Run(name, func(t *testing.T) {
+ actual := AreSameRepos(tc.input1, tc.input2)
+ assert.Equal(t, tc.expected, actual)
+ })
+ }
+} Feedback & Suggestions:
😼 Mergecat review of pkg/repoUrl.go@@ -6,6 +6,8 @@ import (
"strings"
"github.com/chainguard-dev/git-urls"
+ "github.com/pkg/errors"
+ "github.com/rs/zerolog/log"
)
type RepoURL struct {
@@ -41,3 +43,28 @@ func NormalizeRepoUrl(s string) (RepoURL, url.Values, error) {
Path: r.Path,
}, r.Query(), nil
}
+
+func Canonicalize(cloneURL string) (RepoURL, error) {
+ parsed, _, err := NormalizeRepoUrl(cloneURL)
+ if err != nil {
+ return RepoURL{}, errors.Wrap(err, "failed to parse clone url")
+ }
+
+ return parsed, nil
+}
+
+func AreSameRepos(url1, url2 string) bool {
+ repo1, err := Canonicalize(url1)
+ if err != nil {
+ log.Warn().Msgf("failed to canonicalize %q", url1)
+ return false
+ }
+
+ repo2, err := Canonicalize(url2)
+ if err != nil {
+ log.Warn().Msgf("failed to canonicalize %q", url2)
+ return false
+ }
+
+ return repo1 == repo2
+} Feedback & Suggestions:
😼 Mergecat review of pkg/git/repo.go@@ -81,6 +81,7 @@ func (r *Repo) Clone(ctx context.Context) error {
}
}
+ log.Info().Msg("repo has been cloned")
return nil
}
@@ -107,22 +108,22 @@ func (r *Repo) GetRemoteHead() (string, error) {
return branchName, nil
}
-func (r *Repo) MergeIntoTarget(ctx context.Context, sha string) error {
+func (r *Repo) MergeIntoTarget(ctx context.Context, ref string) error {
// Merge the last commit into a tmp branch off of the target branch
_, span := tracer.Start(ctx, "Repo - RepoMergeIntoTarget",
trace.WithAttributes(
attribute.String("branch_name", r.BranchName),
attribute.String("clone_url", r.CloneURL),
attribute.String("directory", r.Directory),
- attribute.String("sha", sha),
+ attribute.String("sha", ref),
))
defer span.End()
- cmd := r.execCommand("git", "merge", sha)
+ cmd := r.execCommand("git", "merge", ref)
out, err := cmd.CombinedOutput()
if err != nil {
telemetry.SetError(span, err, "merge commit into branch")
- log.Error().Err(err).Msgf("unable to merge %s, %s", sha, out)
+ log.Error().Err(err).Msgf("unable to merge %s, %s", ref, out)
return err
}
Feedback & Suggestions:
😼 Mergecat review of pkg/events/runner.go@@ -11,7 +11,6 @@ import (
"github.com/zapier/kubechecks/pkg"
"github.com/zapier/kubechecks/pkg/checks"
"github.com/zapier/kubechecks/pkg/container"
- "github.com/zapier/kubechecks/pkg/git"
"github.com/zapier/kubechecks/pkg/msg"
"github.com/zapier/kubechecks/telemetry"
)
@@ -23,9 +22,13 @@ type Runner struct {
}
func newRunner(
- ctr container.Container, app v1alpha1.Application, repo *git.Repo,
- appName, k8sVersion string, jsonManifests, yamlManifests []string,
- logger zerolog.Logger, note *msg.Message, queueApp, removeApp func(application v1alpha1.Application),
+ ctr container.Container,
+ app v1alpha1.Application,
+ appName, k8sVersion string,
+ jsonManifests, yamlManifests []string,
+ logger zerolog.Logger,
+ note *msg.Message,
+ queueApp, removeApp func(application v1alpha1.Application),
) *Runner {
return &Runner{
Request: checks.Request{
@@ -38,7 +41,6 @@ func newRunner(
Note: note,
QueueApp: queueApp,
RemoveApp: removeApp,
- Repo: repo,
YamlManifests: yamlManifests,
},
} Feedback & Suggestions:
😼 Mergecat review of README.md@@ -7,13 +7,13 @@
## Pull/Merge Request driven checks
-When using ArgoCD, it can be difficult to tell just how your Pull/Merge Request (PR/MR) will impact your live deployment. `kubechecks` was designed to address this problem; every time a new PR/MR is created, `kubechecks` will automatically determine what's changed and how it will impact your `main`/default branchs state, informing you of the details directly on the PR/MR. As a bonus, it also lints and checks your Kubernetes manifests to let you know ahead of time if something is outdated, invalid, or otherwise not good practice.
+When using ArgoCD, it can be difficult to tell just how your Pull/Merge Request (PR/MR) will impact your live deployment. `kubechecks` was designed to address this problem; every time a new PR/MR is created, `kubechecks` will automatically determine what's changed and how it will impact your `main`/default branch's state, informing you of the details directly on the PR/MR. As a bonus, it also lints and checks your Kubernetes manifests to let you know ahead of time if something is outdated, invalid, or otherwise not good practice.
![Demo](./docs/gif/kubechecks.gif)
### How it works
-This tool provides a server function that processes webhooks from Gitlab/Github, clones the repository at the `HEAD` SHA of the PR/MR, and runs various check suites, commenting the output of each check in a single comment on your PR/MR. `kubechecks` talks directly to ArgoCD to get the live state of your deployments to ensure that you have the most accurate information about how your changes will affect your production code.
+This tool provides a server function that processes webhooks from Gitlab/Github, clones the repository at the `HEAD` SHA of the PR/MR, and runs various check suites, commenting the output of each check in a single comment on your PR/MR. `kubechecks` talks directly to ArgoCD to get the live state of your deployments and talks directly to ArgoCD's repo server to generate the new resources to ensure that you have the most accurate information about how your changes will affect your production code.
### Architecture
Feedback & Suggestions:
😼 Mergecat review of pkg/config/config.go type ServerConfig struct {
// argocd
- ArgoCDServerAddr string `mapstructure:"argocd-api-server-addr"`
- ArgoCDToken string `mapstructure:"argocd-api-token"`
- ArgoCDPathPrefix string `mapstructure:"argocd-api-path-prefix"`
- ArgoCDInsecure bool `mapstructure:"argocd-api-insecure"`
- ArgoCDNamespace string `mapstructure:"argocd-api-namespace"`
- ArgoCDPlainText bool `mapstructure:"argocd-api-plaintext"`
- KubernetesConfig string `mapstructure:"kubernetes-config"`
- KubernetesType string `mapstructure:"kubernetes-type"`
- KubernetesClusterID string `mapstructure:"kubernetes-clusterid"`
+ ArgoCDServerAddr string `mapstructure:"argocd-api-server-addr"`
+ ArgoCDToken string `mapstructure:"argocd-api-token"`
+ ArgoCDPathPrefix string `mapstructure:"argocd-api-path-prefix"`
+ ArgoCDInsecure bool `mapstructure:"argocd-api-insecure"`
+ ArgoCDNamespace string `mapstructure:"argocd-api-namespace"`
+ ArgoCDPlainText bool `mapstructure:"argocd-api-plaintext"`
+ ArgoCDRepositoryEndpoint string `mapstructure:"argocd-repository-endpoint"`
+ ArgoCDRepositoryInsecure bool `mapstructure:"argocd-repository-insecure"`
+ ArgoCDSendFullRepository bool `mapstructure:"argocd-send-full-repository"`
+ KubernetesConfig string `mapstructure:"kubernetes-config"`
+ KubernetesType string `mapstructure:"kubernetes-type"`
+ KubernetesClusterID string `mapstructure:"kubernetes-clusterid"`
// otel
EnableOtel bool `mapstructure:"otel-enabled"` Feedback & Suggestions:
😼 Mergecat review of pkg/app_watcher/appset_watcher.go@@ -13,8 +13,8 @@ import (
"github.com/rs/zerolog/log"
"github.com/zapier/kubechecks/pkg/appdir"
"github.com/zapier/kubechecks/pkg/config"
+ "github.com/zapier/kubechecks/pkg/container"
"k8s.io/apimachinery/pkg/util/runtime"
- "k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
)
@@ -28,16 +28,16 @@ type ApplicationSetWatcher struct {
}
// NewApplicationSetWatcher creates new instance of ApplicationWatcher.
-func NewApplicationSetWatcher(kubeCfg *rest.Config, vcsToArgoMap appdir.VcsToArgoMap, cfg config.ServerConfig) (*ApplicationSetWatcher, error) {
- if kubeCfg == nil {
+func NewApplicationSetWatcher(ctr container.Container) (*ApplicationSetWatcher, error) {
+ if ctr.KubeClientSet == nil {
return nil, fmt.Errorf("kubeCfg cannot be nil")
}
ctrl := ApplicationSetWatcher{
- applicationClientset: appclientset.NewForConfigOrDie(kubeCfg),
- vcsToArgoMap: vcsToArgoMap,
+ applicationClientset: appclientset.NewForConfigOrDie(ctr.KubeClientSet.Config()),
+ vcsToArgoMap: ctr.VcsToArgoMap,
}
- appInformer, appLister := ctrl.newApplicationSetInformerAndLister(time.Second*30, cfg)
+ appInformer, appLister := ctrl.newApplicationSetInformerAndLister(time.Second*30, ctr.Config)
ctrl.appInformer = appInformer
ctrl.appLister = appLister Feedback & Suggestions:
Overall, these changes improve the modularity and maintainability of the code. Keep up the good work! 🚀 😼 Mergecat review of pkg/checks/diff/diff.go@@ -259,18 +259,25 @@ func getArgoSettings(ctx context.Context, request checks.Request) (*settings.Set
var nilApp = argoappv1.Application{}
func isApp(item objKeyLiveTarget, manifests []byte) (argoappv1.Application, bool) {
+ logger := log.With().
+ Str("kind", item.key.Kind).
+ Str("name", item.key.Name).
+ Str("namespace", item.key.Namespace).
+ Str("group", item.key.Group).
+ Logger()
+
if strings.ToLower(item.key.Group) != "argoproj.io" {
- log.Debug().Str("group", item.key.Group).Msg("group is not correct")
+ logger.Debug().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")
+ logger.Debug().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")
+ logger.Warn().Err(err).Msg("failed to deserialize application")
return nilApp, false
}
@@ -346,8 +353,9 @@ type resourceInfoProvider struct {
namespacedByGk map[schema.GroupKind]bool
}
-// Infer if obj is namespaced or not from corresponding live objects list. If corresponding live object has namespace then target object is also namespaced.
-// If live object is missing then it does not matter if target is namespaced or not.
+// IsNamespaced infers if obj is namespaced or not from corresponding live objects list. If corresponding live object
+// has namespace then target object is also namespaced. If live object is missing then it does not matter if target is
+// namespaced or not.
func (p *resourceInfoProvider) IsNamespaced(gk schema.GroupKind) (bool, error) {
return p.namespacedByGk[gk], nil
} Feedback & Suggestions:
Overall, these changes improve the code's readability and maintainability. Keep up the good work! 🎉 😼 Mergecat review of pkg/checks/kubeconform/validate_test.go@@ -1,7 +1,6 @@
package kubeconform
import (
- "context"
"fmt"
"os"
"strings"
@@ -16,17 +15,15 @@ import (
)
func TestDefaultGetSchemaLocations(t *testing.T) {
- ctx := context.TODO()
ctr := container.Container{}
- schemaLocations := getSchemaLocations(ctx, ctr, "/some/other/path")
+ schemaLocations := getSchemaLocations(ctr)
// default schema location is "./schemas"
assert.Len(t, schemaLocations, 1)
assert.Equal(t, "default", schemaLocations[0])
}
func TestGetRemoteSchemaLocations(t *testing.T) {
- ctx := context.TODO()
ctr := container.Container{}
if os.Getenv("CI") == "" {
@@ -39,7 +36,7 @@ func TestGetRemoteSchemaLocations(t *testing.T) {
// t.Setenv("KUBECHECKS_SCHEMAS_LOCATION", fixture.URL) // doesn't work because viper needs to initialize from root, which doesn't happen
viper.Set("schemas-location", []string{fixture.URL})
- schemaLocations := getSchemaLocations(ctx, ctr, "/some/other/path")
+ schemaLocations := getSchemaLocations(ctr)
hasTmpDirPrefix := strings.HasPrefix(schemaLocations[0], "/tmp/schemas")
assert.Equal(t, hasTmpDirPrefix, true, "invalid schemas location. Schema location should have prefix /tmp/schemas but has %s", schemaLocations[0])
} Feedback & Suggestions:
Overall, the changes seem to simplify the function calls, but ensure that these simplifications do not compromise the functionality or flexibility of the code. 🛠️ 😼 Mergecat review of mocks/gitlab_client/mocks/mock_ProjectsServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockProjectsServices) AddProjectHook(pid interface{}, opt *gitlab.AddP
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for AddProjectHook")
+ }
+
var r0 *gitlab.ProjectHook
var r1 *gitlab.Response
var r2 error
@@ -111,6 +115,10 @@ func (_m *MockProjectsServices) EditProjectHook(pid interface{}, hook int, opt *
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for EditProjectHook")
+ }
+
var r0 *gitlab.ProjectHook
var r1 *gitlab.Response
var r2 error
@@ -191,6 +199,10 @@ func (_m *MockProjectsServices) GetProject(pid interface{}, opt *gitlab.GetProje
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for GetProject")
+ }
+
var r0 *gitlab.Project
var r1 *gitlab.Response
var r2 error
@@ -270,6 +282,10 @@ func (_m *MockProjectsServices) ListProjectHooks(pid interface{}, opt *gitlab.Li
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for ListProjectHooks")
+ }
+
var r0 []*gitlab.ProjectHook
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of cmd/root.go@@ -85,7 +85,7 @@ func init() {
newStringOpts().
withChoices("hide", "delete").
withDefault("hide"))
- stringSliceFlag(flags, "schemas-location", "Sets schema locations to be used for every check request. Can be common paths inside the repos being checked or git urls in either git or http(s) format.")
+ stringSliceFlag(flags, "schemas-location", "Sets schema locations to be used for every check request. Can be a common path on the host or git urls in either git or http(s) format.")
boolFlag(flags, "enable-conftest", "Set to true to enable conftest policy checking of manifests.")
stringSliceFlag(flags, "policies-location", "Sets rego policy locations to be used for every check request. Can be common path inside the repos being checked or git urls in either git or http(s) format.",
newStringSliceOpts().
@@ -124,9 +124,6 @@ func init() {
}
func setupLogOutput() {
- output := zerolog.ConsoleWriter{Out: os.Stdout}
- log.Logger = log.Output(output)
-
// Default level is info, unless debug flag is present
levelFlag := viper.GetString("log-level")
level, err := zerolog.ParseLevel(levelFlag)
@@ -135,6 +132,10 @@ func setupLogOutput() {
}
zerolog.SetGlobalLevel(level)
+
+ output := zerolog.ConsoleWriter{Out: os.Stdout}
+ log.Logger = log.Output(output)
+
log.Debug().Msg("Debug level logging enabled.")
log.Trace().Msg("Trace level logging enabled.")
log.Info().Msg("Initialized logger.") Feedback & Suggestions:
Overall, these changes seem to improve the clarity and logical flow of the code. Just ensure that the logging behavior is as expected after these modifications. 🛠️ 😼 Mergecat review of mocks/gitlab_client/mocks/mock_NotesServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockNotesServices) CreateMergeRequestNote(pid interface{}, mergeReques
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for CreateMergeRequestNote")
+ }
+
var r0 *gitlab.Note
var r1 *gitlab.Response
var r2 error
@@ -112,6 +116,10 @@ func (_m *MockNotesServices) DeleteMergeRequestNote(pid interface{}, mergeReques
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for DeleteMergeRequestNote")
+ }
+
var r0 *gitlab.Response
var r1 error
if rf, ok := ret.Get(0).(func(interface{}, int, int, ...gitlab.RequestOptionFunc) (*gitlab.Response, error)); ok {
@@ -183,6 +191,10 @@ func (_m *MockNotesServices) ListMergeRequestNotes(pid interface{}, mergeRequest
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for ListMergeRequestNotes")
+ }
+
var r0 []*gitlab.Note
var r1 *gitlab.Response
var r2 error
@@ -263,6 +275,10 @@ func (_m *MockNotesServices) UpdateMergeRequestNote(pid interface{}, mergeReques
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for UpdateMergeRequestNote")
+ }
+
var r0 *gitlab.Note
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of mocks/generator/mocks/mock_Generator.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package generator
@@ -27,6 +27,10 @@ func (_m *MockGenerator) EXPECT() *MockGenerator_Expecter {
func (_m *MockGenerator) GenerateParams(appSetGenerator *v1alpha1.ApplicationSetGenerator, applicationSetInfo *v1alpha1.ApplicationSet) ([]map[string]interface{}, error) {
ret := _m.Called(appSetGenerator, applicationSetInfo)
+ if len(ret) == 0 {
+ panic("no return value specified for GenerateParams")
+ }
+
var r0 []map[string]interface{}
var r1 error
if rf, ok := ret.Get(0).(func(*v1alpha1.ApplicationSetGenerator, *v1alpha1.ApplicationSet) ([]map[string]interface{}, error)); ok {
@@ -82,6 +86,10 @@ func (_c *MockGenerator_GenerateParams_Call) RunAndReturn(run func(*v1alpha1.App
func (_m *MockGenerator) GetRequeueAfter(appSetGenerator *v1alpha1.ApplicationSetGenerator) time.Duration {
ret := _m.Called(appSetGenerator)
+ if len(ret) == 0 {
+ panic("no return value specified for GetRequeueAfter")
+ }
+
var r0 time.Duration
if rf, ok := ret.Get(0).(func(*v1alpha1.ApplicationSetGenerator) time.Duration); ok {
r0 = rf(appSetGenerator)
@@ -124,6 +132,10 @@ func (_c *MockGenerator_GetRequeueAfter_Call) RunAndReturn(run func(*v1alpha1.Ap
func (_m *MockGenerator) GetTemplate(appSetGenerator *v1alpha1.ApplicationSetGenerator) *v1alpha1.ApplicationSetTemplate {
ret := _m.Called(appSetGenerator)
+ if len(ret) == 0 {
+ panic("no return value specified for GetTemplate")
+ }
+
var r0 *v1alpha1.ApplicationSetTemplate
if rf, ok := ret.Get(0).(func(*v1alpha1.ApplicationSetGenerator) *v1alpha1.ApplicationSetTemplate); ok {
r0 = rf(appSetGenerator) Feedback & Suggestions:
😼 Mergecat review of mocks/gitlab_client/mocks/mock_MergeRequestsServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package gitlab_client
@@ -32,6 +32,10 @@ func (_m *MockMergeRequestsServices) GetMergeRequest(pid interface{}, mergeReque
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for GetMergeRequest")
+ }
+
var r0 *gitlab.MergeRequest
var r1 *gitlab.Response
var r2 error
@@ -112,6 +116,10 @@ func (_m *MockMergeRequestsServices) GetMergeRequestChanges(pid interface{}, mer
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for GetMergeRequestChanges")
+ }
+
var r0 *gitlab.MergeRequest
var r1 *gitlab.Response
var r2 error
@@ -192,6 +200,10 @@ func (_m *MockMergeRequestsServices) GetMergeRequestDiffVersions(pid interface{}
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for GetMergeRequestDiffVersions")
+ }
+
var r0 []*gitlab.MergeRequestDiffVersion
var r1 *gitlab.Response
var r2 error
@@ -272,6 +284,10 @@ func (_m *MockMergeRequestsServices) ListMergeRequestDiffs(pid interface{}, merg
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for ListMergeRequestDiffs")
+ }
+
var r0 []*gitlab.MergeRequestDiff
var r1 *gitlab.Response
var r2 error
@@ -352,6 +368,10 @@ func (_m *MockMergeRequestsServices) UpdateMergeRequest(pid interface{}, mergeRe
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
+ if len(ret) == 0 {
+ panic("no return value specified for UpdateMergeRequest")
+ }
+
var r0 *gitlab.MergeRequest
var r1 *gitlab.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of mocks/affected_apps/mocks/mock_argoClient.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package affected_apps
@@ -26,6 +26,10 @@ func (_m *MockargoClient) EXPECT() *MockargoClient_Expecter {
func (_m *MockargoClient) GetApplications(ctx context.Context) (*v1alpha1.ApplicationList, error) {
ret := _m.Called(ctx)
+ if len(ret) == 0 {
+ panic("no return value specified for GetApplications")
+ }
+
var r0 *v1alpha1.ApplicationList
var r1 error
if rf, ok := ret.Get(0).(func(context.Context) (*v1alpha1.ApplicationList, error)); ok {
@@ -80,6 +84,10 @@ func (_c *MockargoClient_GetApplications_Call) RunAndReturn(run func(context.Con
func (_m *MockargoClient) GetApplicationsByAppset(ctx context.Context, appsetName string) (*v1alpha1.ApplicationList, error) {
ret := _m.Called(ctx, appsetName)
+ if len(ret) == 0 {
+ panic("no return value specified for GetApplicationsByAppset")
+ }
+
var r0 *v1alpha1.ApplicationList
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, string) (*v1alpha1.ApplicationList, error)); ok { Feedback & Suggestions:
😼 Mergecat review of mocks/github_client/mocks/mock_PullRequestsServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package github_client
@@ -27,6 +27,10 @@ func (_m *MockPullRequestsServices) EXPECT() *MockPullRequestsServices_Expecter
func (_m *MockPullRequestsServices) Get(ctx context.Context, owner string, repo string, number int) (*github.PullRequest, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, number)
+ if len(ret) == 0 {
+ panic("no return value specified for Get")
+ }
+
var r0 *github.PullRequest
var r1 *github.Response
var r2 error
@@ -93,6 +97,10 @@ func (_c *MockPullRequestsServices_Get_Call) RunAndReturn(run func(context.Conte
func (_m *MockPullRequestsServices) GetRaw(ctx context.Context, owner string, repo string, number int, opts github.RawOptions) (string, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, number, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for GetRaw")
+ }
+
var r0 string
var r1 *github.Response
var r2 error
@@ -158,6 +166,10 @@ func (_c *MockPullRequestsServices_GetRaw_Call) RunAndReturn(run func(context.Co
func (_m *MockPullRequestsServices) List(ctx context.Context, owner string, repo string, opts *github.PullRequestListOptions) ([]*github.PullRequest, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for List")
+ }
+
var r0 []*github.PullRequest
var r1 *github.Response
var r2 error
@@ -224,6 +236,10 @@ func (_c *MockPullRequestsServices_List_Call) RunAndReturn(run func(context.Cont
func (_m *MockPullRequestsServices) ListFiles(ctx context.Context, owner string, repo string, number int, opts *github.ListOptions) ([]*github.CommitFile, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, number, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for ListFiles")
+ }
+
var r0 []*github.CommitFile
var r1 *github.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of docs/usage.md@@ -41,6 +41,9 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_ARGOCD_API_PLAINTEXT`|Enable to use plaintext connections without TLS.|`false`|
|`KUBECHECKS_ARGOCD_API_SERVER_ADDR`|ArgoCD API Server Address.|`argocd-server`|
|`KUBECHECKS_ARGOCD_API_TOKEN`|ArgoCD API token.||
+|`KUBECHECKS_ARGOCD_REPOSITORY_ENDPOINT`|Location of the argocd repository service endpoint.|`argocd-repo-server.argocd:8081`|
+|`KUBECHECKS_ARGOCD_REPOSITORY_INSECURE`|True if you need to skip validating the grpc tls certificate.|`true`|
+|`KUBECHECKS_ARGOCD_SEND_FULL_REPOSITORY`|Set to true if you want to try to send the full repository to ArgoCD when generating manifests.|`false`|
|`KUBECHECKS_ENABLE_CONFTEST`|Set to true to enable conftest policy checking of manifests.|`false`|
|`KUBECHECKS_ENABLE_HOOKS_RENDERER`|Render hooks.|`true`|
|`KUBECHECKS_ENABLE_KUBECONFORM`|Enable kubeconform checks.|`true`|
@@ -66,7 +69,7 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_POLICIES_LOCATION`|Sets rego policy locations to be used for every check request. Can be common path inside the repos being checked or git urls in either git or http(s) format.|`[./policies]`|
|`KUBECHECKS_REPLAN_COMMENT_MSG`|comment message which re-triggers kubechecks on PR.|`kubechecks again`|
|`KUBECHECKS_REPO_REFRESH_INTERVAL`|Interval between static repo refreshes (for schemas and policies).|`5m`|
-|`KUBECHECKS_SCHEMAS_LOCATION`|Sets schema locations to be used for every check request. Can be common paths inside the repos being checked or git urls in either git or http(s) format.|`[]`|
+|`KUBECHECKS_SCHEMAS_LOCATION`|Sets schema locations to be used for every check request. Can be a common path on the host or git urls in either git or http(s) format.|`[]`|
|`KUBECHECKS_SHOW_DEBUG_INFO`|Set to true to print debug info to the footer of MR comments.|`false`|
|`KUBECHECKS_TIDY_OUTDATED_COMMENTS_MODE`|Sets the mode to use when tidying outdated comments. One of hide, delete.|`hide`|
|`KUBECHECKS_VCS_BASE_URL`|VCS base url, useful if self hosting gitlab, enterprise github, etc.|| Feedback & Suggestions:
😼 Mergecat review of mocks/github_client/mocks/mock_IssuesServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package github_client
@@ -27,6 +27,10 @@ func (_m *MockIssuesServices) EXPECT() *MockIssuesServices_Expecter {
func (_m *MockIssuesServices) CreateComment(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, number, comment)
+ if len(ret) == 0 {
+ panic("no return value specified for CreateComment")
+ }
+
var r0 *github.IssueComment
var r1 *github.Response
var r2 error
@@ -94,6 +98,10 @@ func (_c *MockIssuesServices_CreateComment_Call) RunAndReturn(run func(context.C
func (_m *MockIssuesServices) DeleteComment(ctx context.Context, owner string, repo string, commentID int64) (*github.Response, error) {
ret := _m.Called(ctx, owner, repo, commentID)
+ if len(ret) == 0 {
+ panic("no return value specified for DeleteComment")
+ }
+
var r0 *github.Response
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, string, string, int64) (*github.Response, error)); ok {
@@ -151,6 +159,10 @@ func (_c *MockIssuesServices_DeleteComment_Call) RunAndReturn(run func(context.C
func (_m *MockIssuesServices) EditComment(ctx context.Context, owner string, repo string, commentID int64, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, commentID, comment)
+ if len(ret) == 0 {
+ panic("no return value specified for EditComment")
+ }
+
var r0 *github.IssueComment
var r1 *github.Response
var r2 error
@@ -218,6 +230,10 @@ func (_c *MockIssuesServices_EditComment_Call) RunAndReturn(run func(context.Con
func (_m *MockIssuesServices) ListComments(ctx context.Context, owner string, repo string, number int, opts *github.IssueListCommentsOptions) ([]*github.IssueComment, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, number, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for ListComments")
+ }
+
var r0 []*github.IssueComment
var r1 *github.Response
var r2 error Feedback & Suggestions:
😼 Mergecat review of pkg/affected_apps/argocd_matcher.go@@ -6,7 +6,6 @@ import (
"github.com/rs/zerolog/log"
"github.com/zapier/kubechecks/pkg/appdir"
- "github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/git"
)
@@ -15,7 +14,7 @@ type ArgocdMatcher struct {
appSetsDirectory *appdir.AppSetDirectory
}
-func NewArgocdMatcher(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) (*ArgocdMatcher, error) {
+func NewArgocdMatcher(vcsToArgoMap appdir.VcsToArgoMap, repo *git.Repo) (*ArgocdMatcher, error) {
repoApps := getArgocdApps(vcsToArgoMap, repo)
kustomizeAppFiles := getKustomizeApps(vcsToArgoMap, repo, repo.Directory)
@@ -41,7 +40,7 @@ func logCounts(repoApps *appdir.AppDirectory) {
}
}
-func getKustomizeApps(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo, repoPath string) *appdir.AppDirectory {
+func getKustomizeApps(vcsToArgoMap appdir.VcsToArgoMap, repo *git.Repo, repoPath string) *appdir.AppDirectory {
log.Debug().Msgf("creating fs for %s", repoPath)
fs := os.DirFS(repoPath)
log.Debug().Msg("following kustomize apps")
@@ -51,15 +50,15 @@ func getKustomizeApps(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo, repoP
return kustomizeAppFiles
}
-func getArgocdApps(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) *appdir.AppDirectory {
+func getArgocdApps(vcsToArgoMap appdir.VcsToArgoMap, repo *git.Repo) *appdir.AppDirectory {
log.Debug().Msgf("looking for %s repos", repo.CloneURL)
repoApps := vcsToArgoMap.GetAppsInRepo(repo.CloneURL)
logCounts(repoApps)
return repoApps
}
-func getArgocdAppSets(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) *appdir.AppSetDirectory {
+func getArgocdAppSets(vcsToArgoMap appdir.VcsToArgoMap, repo *git.Repo) *appdir.AppSetDirectory {
log.Debug().Msgf("looking for %s repos", repo.CloneURL)
repoApps := vcsToArgoMap.GetAppSetsInRepo(repo.CloneURL)
Feedback & Suggestions:
😼 Mergecat review of cmd/process.go@@ -1,8 +1,13 @@
package cmd
import (
+ "os"
+ "path/filepath"
+
+ "github.com/argoproj/argo-cd/v2/common"
"github.com/rs/zerolog/log"
"github.com/spf13/cobra"
+ "github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/config"
"github.com/zapier/kubechecks/pkg/server"
@@ -15,14 +20,42 @@ var processCmd = &cobra.Command{
Run: func(cmd *cobra.Command, args []string) {
ctx := cmd.Context()
+ tempPath, err := os.MkdirTemp("", "")
+ if err != nil {
+ log.Fatal().Err(err).Msg("fail to create ssh data dir")
+ }
+ defer func() {
+ os.RemoveAll(tempPath)
+ }()
+
+ // symlink local ssh known hosts to argocd ssh known hosts
+ homeDir, err := os.UserHomeDir()
+ if err != nil {
+ log.Fatal().Err(err).Msg("failed to get user home dir")
+ }
+ source := filepath.Join(homeDir, ".ssh", "known_hosts")
+ target := filepath.Join(tempPath, common.DefaultSSHKnownHostsName)
+
+ if err := os.Symlink(source, target); err != nil {
+ log.Fatal().Err(err).Msg("fail to symlink ssh_known_hosts file")
+ }
+
+ if err := os.Setenv("ARGOCD_SSH_DATA_PATH", tempPath); err != nil {
+ log.Fatal().Err(err).Msg("fail to set ARGOCD_SSH_DATA_PATH")
+ }
+
cfg, err := config.New()
if err != nil {
log.Fatal().Err(err).Msg("failed to generate config")
}
- ctr, err := newContainer(ctx, cfg, false)
+ if len(args) != 1 {
+ log.Fatal().Msg("usage: kubechecks process PR_REF")
+ }
+
+ ctr, err := container.New(ctx, cfg)
if err != nil {
- log.Fatal().Err(err).Msg("failed to create container")
+ log.Fatal().Err(err).Msg("failed to create clients")
}
log.Info().Msg("initializing git settings") Feedback & Suggestions:
😼 Mergecat review of pkg/appdir/vcstoargomap.go@@ -79,37 +79,39 @@ func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, fs fs.FS) *AppDirecto
return result
}
-func (v2a VcsToArgoMap) AddApp(app *v1alpha1.Application) {
- if app.Spec.Source == nil {
- log.Warn().Msgf("%s/%s: no source, skipping", app.Namespace, app.Name)
- return
+func (v2a VcsToArgoMap) processApp(app v1alpha1.Application, fn func(*AppDirectory)) {
+
+ if src := app.Spec.Source; src != nil {
+ appDirectory := v2a.GetAppsInRepo(src.RepoURL)
+ fn(appDirectory)
}
- appDirectory := v2a.GetAppsInRepo(app.Spec.Source.RepoURL)
- appDirectory.ProcessApp(*app)
+ for _, src := range app.Spec.Sources {
+ appDirectory := v2a.GetAppsInRepo(src.RepoURL)
+ fn(appDirectory)
+ }
}
-func (v2a VcsToArgoMap) UpdateApp(old *v1alpha1.Application, new *v1alpha1.Application) {
- if new.Spec.Source == nil {
- log.Warn().Msgf("%s/%s: no source, skipping", new.Namespace, new.Name)
- return
- }
+func (v2a VcsToArgoMap) AddApp(app *v1alpha1.Application) {
+ v2a.processApp(*app, func(directory *AppDirectory) {
+ directory.AddApp(*app)
+ })
+}
- oldAppDirectory := v2a.GetAppsInRepo(old.Spec.Source.RepoURL)
- oldAppDirectory.RemoveApp(*old)
+func (v2a VcsToArgoMap) UpdateApp(old *v1alpha1.Application, new *v1alpha1.Application) {
+ v2a.processApp(*old, func(directory *AppDirectory) {
+ directory.RemoveApp(*old)
+ })
- newAppDirectory := v2a.GetAppsInRepo(new.Spec.Source.RepoURL)
- newAppDirectory.ProcessApp(*new)
+ v2a.processApp(*new, func(directory *AppDirectory) {
+ directory.AddApp(*new)
+ })
}
func (v2a VcsToArgoMap) DeleteApp(app *v1alpha1.Application) {
- if app.Spec.Source == nil {
- log.Warn().Msgf("%s/%s: no source, skipping", app.Namespace, app.Name)
- return
- }
-
- oldAppDirectory := v2a.GetAppsInRepo(app.Spec.Source.RepoURL)
- oldAppDirectory.RemoveApp(*app)
+ v2a.processApp(*app, func(directory *AppDirectory) {
+ directory.RemoveApp(*app)
+ })
}
func (v2a VcsToArgoMap) GetVcsRepos() []string { Feedback & Suggestions:
😼 Mergecat review of mocks/github_client/mocks/mock_RepositoriesServices.go@@ -1,4 +1,4 @@
-// Code generated by mockery v2.37.1. DO NOT EDIT.
+// Code generated by mockery v2.46.3. DO NOT EDIT.
package github_client
@@ -27,6 +27,10 @@ func (_m *MockRepositoriesServices) EXPECT() *MockRepositoriesServices_Expecter
func (_m *MockRepositoriesServices) CreateHook(ctx context.Context, owner string, repo string, hook *github.Hook) (*github.Hook, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, hook)
+ if len(ret) == 0 {
+ panic("no return value specified for CreateHook")
+ }
+
var r0 *github.Hook
var r1 *github.Response
var r2 error
@@ -93,6 +97,10 @@ func (_c *MockRepositoriesServices_CreateHook_Call) RunAndReturn(run func(contex
func (_m *MockRepositoriesServices) CreateStatus(ctx context.Context, owner string, repo string, ref string, status *github.RepoStatus) (*github.RepoStatus, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, ref, status)
+ if len(ret) == 0 {
+ panic("no return value specified for CreateStatus")
+ }
+
var r0 *github.RepoStatus
var r1 *github.Response
var r2 error
@@ -160,6 +168,10 @@ func (_c *MockRepositoriesServices_CreateStatus_Call) RunAndReturn(run func(cont
func (_m *MockRepositoriesServices) Get(ctx context.Context, owner string, repo string) (*github.Repository, *github.Response, error) {
ret := _m.Called(ctx, owner, repo)
+ if len(ret) == 0 {
+ panic("no return value specified for Get")
+ }
+
var r0 *github.Repository
var r1 *github.Response
var r2 error
@@ -225,6 +237,10 @@ func (_c *MockRepositoriesServices_Get_Call) RunAndReturn(run func(context.Conte
func (_m *MockRepositoriesServices) GetContents(ctx context.Context, owner string, repo string, path string, opts *github.RepositoryContentGetOptions) (*github.RepositoryContent, []*github.RepositoryContent, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, path, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for GetContents")
+ }
+
var r0 *github.RepositoryContent
var r1 []*github.RepositoryContent
var r2 *github.Response
@@ -301,6 +317,10 @@ func (_c *MockRepositoriesServices_GetContents_Call) RunAndReturn(run func(conte
func (_m *MockRepositoriesServices) ListHooks(ctx context.Context, owner string, repo string, opts *github.ListOptions) ([]*github.Hook, *github.Response, error) {
ret := _m.Called(ctx, owner, repo, opts)
+ if len(ret) == 0 {
+ panic("no return value specified for ListHooks")
+ }
+
var r0 []*github.Hook
var r1 *github.Response
var r2 error Feedback & Suggestions:
By addressing these points, you can improve the stability and maintainability of your code. Happy coding! 😺 😼 Mergecat review of pkg/checks/kubeconform/validate.go@@ -5,7 +5,6 @@ import (
"fmt"
"io"
"os"
- "path/filepath"
"strings"
"github.com/pkg/errors"
@@ -20,7 +19,7 @@ import (
var tracer = otel.Tracer("pkg/checks/kubeconform")
-func getSchemaLocations(ctx context.Context, ctr container.Container, tempRepoPath string) []string {
+func getSchemaLocations(ctr container.Container) []string {
cfg := ctr.Config
locations := []string{
@@ -29,28 +28,13 @@ func getSchemaLocations(ctx context.Context, ctr container.Container, tempRepoPa
}
// schemas configured globally
- for _, schemasLocation := range cfg.SchemasLocations {
- if strings.HasPrefix(schemasLocation, "http://") || strings.HasPrefix(schemasLocation, "https://") {
- locations = append(locations, schemasLocation)
- } else {
- if !filepath.IsAbs(schemasLocation) {
- schemasLocation = filepath.Join(tempRepoPath, schemasLocation)
- }
-
- if _, err := os.Stat(schemasLocation); err != nil {
- log.Warn().
- Err(err).
- Str("path", schemasLocation).
- Msg("schemas location is invalid, skipping")
- } else {
- locations = append(locations, schemasLocation)
- }
- }
- }
+ locations = append(locations, cfg.SchemasLocations...)
for index := range locations {
location := locations[index]
+ oldLocation := location
if location == "default" || strings.Contains(location, "{{") {
+ log.Debug().Str("location", location).Msg("location requires no processing to be valid")
continue
}
@@ -60,12 +44,14 @@ func getSchemaLocations(ctx context.Context, ctr container.Container, tempRepoPa
location += "{{ .NormalizedKubernetesVersion }}/{{ .ResourceKind }}{{ .KindSuffix }}.json"
locations[index] = location
+
+ log.Debug().Str("old", oldLocation).Str("new", location).Msg("processed schema location")
}
return locations
}
-func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, targetKubernetesVersion, tempRepoPath string, appManifests []string) (msg.Result, error) {
+func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, targetKubernetesVersion string, appManifests []string) (msg.Result, error) {
_, span := tracer.Start(ctx, "ArgoCdAppValidate")
defer span.End()
@@ -92,7 +78,7 @@ func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, ta
var (
outputString []string
- schemaLocations = getSchemaLocations(ctx, ctr, tempRepoPath)
+ schemaLocations = getSchemaLocations(ctr)
)
log.Debug().Msgf("cache location: %s", vOpts.Cache) Feedback & Suggestions:
Overall, the refactoring improves code simplicity but requires careful consideration of security and functionality impacts. 🛡️🔍 😼 Mergecat review of pkg/app_watcher/app_watcher.go@@ -12,11 +12,11 @@ import (
informers "github.com/argoproj/argo-cd/v2/pkg/client/informers/externalversions/application/v1alpha1"
applisters "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
"github.com/rs/zerolog/log"
+ "github.com/zapier/kubechecks/pkg/appdir"
+ "github.com/zapier/kubechecks/pkg/container"
"k8s.io/apimachinery/pkg/util/runtime"
- "k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
- "github.com/zapier/kubechecks/pkg/appdir"
"github.com/zapier/kubechecks/pkg/config"
)
@@ -34,16 +34,16 @@ type ApplicationWatcher struct {
// - kubeCfg is the Kubernetes configuration.
// - vcsToArgoMap is the mapping between VCS and Argo applications.
// - cfg is the server configuration.
-func NewApplicationWatcher(kubeCfg *rest.Config, vcsToArgoMap appdir.VcsToArgoMap, cfg config.ServerConfig) (*ApplicationWatcher, error) {
- if kubeCfg == nil {
+func NewApplicationWatcher(ctr container.Container) (*ApplicationWatcher, error) {
+ if ctr.KubeClientSet == nil {
return nil, fmt.Errorf("kubeCfg cannot be nil")
}
ctrl := ApplicationWatcher{
- applicationClientset: appclientset.NewForConfigOrDie(kubeCfg),
- vcsToArgoMap: vcsToArgoMap,
+ applicationClientset: appclientset.NewForConfigOrDie(ctr.KubeClientSet.Config()),
+ vcsToArgoMap: ctr.VcsToArgoMap,
}
- appInformer, appLister := ctrl.newApplicationInformerAndLister(time.Second*30, cfg)
+ appInformer, appLister := ctrl.newApplicationInformerAndLister(time.Second*30, ctr.Config)
ctrl.appInformer = appInformer
ctrl.appLister = appLister
@@ -152,14 +152,14 @@ func canProcessApp(obj interface{}) (*appv1alpha1.Application, bool) {
return nil, false
}
- for _, src := range app.Spec.Sources {
+ if src := app.Spec.Source; src != nil {
if isGitRepo(src.RepoURL) {
return app, true
}
}
- if app.Spec.Source != nil {
- if isGitRepo(app.Spec.Source.RepoURL) {
+ for _, src := range app.Spec.Sources {
+ if isGitRepo(src.RepoURL) {
return app, true
}
} Feedback & Suggestions:
😼 Mergecat review of pkg/events/check.go@@ -42,7 +42,7 @@ type CheckEvent struct {
repoManager repoManager
processors []checks.ProcessorEntry
repoLock sync.Mutex
- clonedRepos map[string]*git.Repo
+ clonedRepos map[repoKey]*git.Repo
addedAppsSet map[string]v1alpha1.Application
addedAppsSetLock sync.Mutex
@@ -82,7 +82,7 @@ func NewCheckEvent(pullRequest vcs.PullRequest, ctr container.Container, repoMan
addedAppsSet: make(map[string]v1alpha1.Application),
appChannel: make(chan *v1alpha1.Application, ctr.Config.MaxQueueSize),
ctr: ctr,
- clonedRepos: make(map[string]*git.Repo),
+ clonedRepos: make(map[repoKey]*git.Repo),
processors: processors,
pullRequest: pullRequest,
repoManager: repoManager,
@@ -160,15 +160,14 @@ func canonicalize(cloneURL string) (pkg.RepoURL, error) {
return parsed, nil
}
-func generateRepoKey(cloneURL pkg.RepoURL, branchName string) string {
- return fmt.Sprintf("%s|||%s", cloneURL.CloneURL(""), branchName)
-}
+type repoKey string
-type hasUsername interface {
- Username() string
+func generateRepoKey(cloneURL pkg.RepoURL, branchName string) repoKey {
+ key := fmt.Sprintf("%s|||%s", cloneURL.CloneURL(""), branchName)
+ return repoKey(key)
}
-func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneURL, branchName string) (*git.Repo, error) {
+func (ce *CheckEvent) getRepo(ctx context.Context, cloneURL, branchName string) (*git.Repo, error) {
var (
err error
repo *git.Repo
@@ -181,7 +180,7 @@ func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneU
if err != nil {
return nil, errors.Wrap(err, "failed to parse clone url")
}
- cloneURL = parsed.CloneURL(vcsClient.Username())
+ cloneURL = parsed.CloneURL(ce.ctr.VcsClient.Username())
branchName = strings.TrimSpace(branchName)
if branchName == "" {
@@ -227,20 +226,36 @@ func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneU
return repo, nil
}
+func (ce *CheckEvent) mergeIntoTarget(ctx context.Context, repo *git.Repo, branch string) error {
+ if err := repo.MergeIntoTarget(ctx, fmt.Sprintf("origin/%s", branch)); err != nil {
+ return errors.Wrap(err, "failed to merge into target")
+ }
+
+ parsed, err := canonicalize(repo.CloneURL)
+ if err != nil {
+ return errors.Wrap(err, "failed to canonicalize url")
+ }
+
+ reposKey := generateRepoKey(parsed, branch)
+ ce.clonedRepos[reposKey] = repo
+
+ return nil
+}
+
func (ce *CheckEvent) Process(ctx context.Context) error {
start := time.Now()
_, span := tracer.Start(ctx, "GenerateListOfAffectedApps")
defer span.End()
// Clone the repo's BaseRef (main, etc.) locally into the temp dir we just made
- repo, err := ce.getRepo(ctx, ce.ctr.VcsClient, ce.pullRequest.CloneURL, ce.pullRequest.BaseRef)
+ repo, err := ce.getRepo(ctx, ce.pullRequest.CloneURL, ce.pullRequest.BaseRef)
if err != nil {
return errors.Wrap(err, "failed to clone repo")
}
// Merge the most recent changes into the branch we just cloned
- if err = repo.MergeIntoTarget(ctx, ce.pullRequest.SHA); err != nil {
+ if err = ce.mergeIntoTarget(ctx, repo, ce.pullRequest.HeadRef); err != nil {
return errors.Wrap(err, "failed to merge into target")
}
@@ -275,11 +290,12 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
for num := 0; num <= ce.ctr.Config.MaxConcurrenctChecks; num++ {
w := worker{
- appChannel: ce.appChannel,
- ctr: ce.ctr,
- logger: ce.logger.With().Int("workerID", num).Logger(),
- processors: ce.processors,
- vcsNote: ce.vcsNote,
+ appChannel: ce.appChannel,
+ ctr: ce.ctr,
+ logger: ce.logger.With().Int("workerID", num).Logger(),
+ pullRequest: ce.pullRequest,
+ processors: ce.processors,
+ vcsNote: ce.vcsNote,
done: ce.wg.Done,
getRepo: ce.getRepo, Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/applications.go@@ -24,11 +24,11 @@ var ErrNoVersionFound = errors.New("no kubernetes version found")
// GetApplicationByName takes a context and a name, then queries the Argo Application client to retrieve the Application with the specified name.
// It returns the found Application and any error encountered during the process.
// If successful, the Application client connection is closed before returning.
-func (argo *ArgoClient) GetApplicationByName(ctx context.Context, name string) (*v1alpha1.Application, error) {
+func (a *ArgoClient) GetApplicationByName(ctx context.Context, name string) (*v1alpha1.Application, error) {
ctx, span := tracer.Start(ctx, "GetApplicationByName")
defer span.End()
- closer, appClient := argo.GetApplicationClient()
+ closer, appClient := a.GetApplicationClient()
defer closer.Close()
resp, err := appClient.Get(ctx, &application.ApplicationQuery{Name: &name})
@@ -43,7 +43,7 @@ func (argo *ArgoClient) GetApplicationByName(ctx context.Context, name string) (
// 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) GetKubernetesVersionByApplication(ctx context.Context, app v1alpha1.Application) (string, error) {
+func (a *ArgoClient) GetKubernetesVersionByApplication(ctx context.Context, app v1alpha1.Application) (string, error) {
ctx, span := tracer.Start(ctx, "GetKubernetesVersionByApplicationName")
defer span.End()
@@ -58,7 +58,7 @@ func (argo *ArgoClient) GetKubernetesVersionByApplication(ctx context.Context, a
}
// Get cluster client
- clusterCloser, clusterClient := argo.GetClusterClient()
+ clusterCloser, clusterClient := a.GetClusterClient()
defer clusterCloser.Close()
// Get cluster
@@ -85,11 +85,11 @@ func (argo *ArgoClient) GetKubernetesVersionByApplication(ctx context.Context, a
// GetApplicationsByLabels takes a context and a labelselector, then queries the Argo Application client to retrieve the Applications with the specified label.
// It returns the found ApplicationList and any error encountered during the process.
// If successful, the Application client connection is closed before returning.
-func (argo *ArgoClient) GetApplicationsByLabels(ctx context.Context, labels string) (*v1alpha1.ApplicationList, error) {
+func (a *ArgoClient) GetApplicationsByLabels(ctx context.Context, labels string) (*v1alpha1.ApplicationList, error) {
ctx, span := tracer.Start(ctx, "GetApplicationsByLabels")
defer span.End()
- closer, appClient := argo.GetApplicationClient()
+ closer, appClient := a.GetApplicationClient()
defer closer.Close()
resp, err := appClient.List(ctx, &application.ApplicationQuery{Selector: &labels})
@@ -103,31 +103,31 @@ func (argo *ArgoClient) GetApplicationsByLabels(ctx context.Context, labels stri
// GetApplicationsByAppset takes a context and an appset, then queries the Argo Application client to retrieve the Applications managed by the appset
// It returns the found ApplicationList and any error encountered during the process.
-func (argo *ArgoClient) GetApplicationsByAppset(ctx context.Context, name string) (*v1alpha1.ApplicationList, error) {
+func (a *ArgoClient) GetApplicationsByAppset(ctx context.Context, name string) (*v1alpha1.ApplicationList, error) {
appsetLabelSelector := "argocd.argoproj.io/application-set-name=" + name
- return argo.GetApplicationsByLabels(ctx, appsetLabelSelector)
+ return a.GetApplicationsByLabels(ctx, appsetLabelSelector)
}
-func (argo *ArgoClient) GetApplications(ctx context.Context) (*v1alpha1.ApplicationList, error) {
+func (a *ArgoClient) GetApplications(ctx context.Context) (*v1alpha1.ApplicationList, error) {
ctx, span := tracer.Start(ctx, "GetApplications")
defer span.End()
- closer, appClient := argo.GetApplicationClient()
+ closer, appClient := a.GetApplicationClient()
defer closer.Close()
resp, err := appClient.List(ctx, new(application.ApplicationQuery))
if err != nil {
telemetry.SetError(span, err, "Argo List All Applications error")
- return nil, errors.Wrap(err, "failed to applications")
+ return nil, errors.Wrap(err, "failed to list applications")
}
return resp, nil
}
-func (argo *ArgoClient) GetApplicationSets(ctx context.Context) (*v1alpha1.ApplicationSetList, error) {
+func (a *ArgoClient) GetApplicationSets(ctx context.Context) (*v1alpha1.ApplicationSetList, error) {
ctx, span := tracer.Start(ctx, "GetApplications")
defer span.End()
- closer, appClient := argo.GetApplicationSetClient()
+ closer, appClient := a.GetApplicationSetClient()
defer closer.Close()
resp, err := appClient.List(ctx, new(applicationset.ApplicationSetListQuery)) Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/client.go package argo_client
import (
+ "crypto/tls"
"io"
- "sync"
"github.com/argoproj/argo-cd/v2/pkg/apiclient"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/settings"
+ repoapiclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient"
+ "github.com/pkg/errors"
"github.com/rs/zerolog/log"
+ client "github.com/zapier/kubechecks/pkg/kubernetes"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/credentials"
+ "k8s.io/client-go/kubernetes"
+ "k8s.io/client-go/rest"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
@@ -18,10 +25,17 @@ import (
type ArgoClient struct {
client apiclient.Client
- manifestsLock sync.Mutex
+ repoClient repoapiclient.RepoServerServiceClient
+ namespace string
+ k8s kubernetes.Interface
+ k8sConfig *rest.Config
+ sendFullRepository bool
}
-func NewArgoClient(cfg config.ServerConfig) (*ArgoClient, error) {
+func NewArgoClient(
+ cfg config.ServerConfig,
+ k8s client.Interface,
+) (*ArgoClient, error) {
opts := &apiclient.ClientOptions{
ServerAddr: cfg.ArgoCDServerAddr,
AuthToken: cfg.ArgoCDToken,
@@ -42,38 +56,54 @@ func NewArgoClient(cfg config.ServerConfig) (*ArgoClient, error) {
return nil, err
}
+ log.Info().Msg("creating client")
+ tlsConfig := tls.Config{InsecureSkipVerify: cfg.ArgoCDRepositoryInsecure}
+ conn, err := grpc.NewClient(cfg.ArgoCDRepositoryEndpoint,
+ grpc.WithTransportCredentials(
+ credentials.NewTLS(&tlsConfig),
+ ),
+ )
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to create client")
+ }
+
return &ArgoClient{
- client: argo,
+ repoClient: repoapiclient.NewRepoServerServiceClient(conn),
+ client: argo,
+ namespace: cfg.ArgoCDNamespace,
+ k8s: k8s.ClientSet(),
+ k8sConfig: k8s.Config(),
+ sendFullRepository: cfg.ArgoCDSendFullRepository,
}, nil
}
// GetApplicationClient has related argocd diff code https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L899
-func (argo *ArgoClient) GetApplicationClient() (io.Closer, application.ApplicationServiceClient) {
- closer, appClient, err := argo.client.NewApplicationClient()
+func (a *ArgoClient) GetApplicationClient() (io.Closer, application.ApplicationServiceClient) {
+ closer, appClient, err := a.client.NewApplicationClient()
if err != nil {
log.Fatal().Err(err).Msg("could not create ArgoCD Application Client")
}
return closer, appClient
}
-func (argo *ArgoClient) GetApplicationSetClient() (io.Closer, applicationset.ApplicationSetServiceClient) {
- closer, appClient, err := argo.client.NewApplicationSetClient()
+func (a *ArgoClient) GetApplicationSetClient() (io.Closer, applicationset.ApplicationSetServiceClient) {
+ closer, appClient, err := a.client.NewApplicationSetClient()
if err != nil {
log.Fatal().Err(err).Msg("could not create ArgoCD Application Set Client")
}
return closer, appClient
}
-func (argo *ArgoClient) GetSettingsClient() (io.Closer, settings.SettingsServiceClient) {
- closer, appClient, err := argo.client.NewSettingsClient()
+func (a *ArgoClient) GetSettingsClient() (io.Closer, settings.SettingsServiceClient) {
+ closer, appClient, err := a.client.NewSettingsClient()
if err != nil {
log.Fatal().Err(err).Msg("could not create ArgoCD Settings Client")
}
return closer, appClient
}
-func (argo *ArgoClient) GetClusterClient() (io.Closer, cluster.ClusterServiceClient) {
- closer, clusterClient, err := argo.client.NewClusterClient()
+func (a *ArgoClient) GetClusterClient() (io.Closer, cluster.ClusterServiceClient) {
+ closer, clusterClient, err := a.client.NewClusterClient()
if err != nil {
log.Fatal().Err(err).Msg("could not create ArgoCD Cluster Client")
} Feedback & Suggestions:
😼 Mergecat review of pkg/container/main.go@@ -2,13 +2,14 @@ package container
import (
"context"
- "io/fs"
+ "fmt"
- "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ "github.com/pkg/errors"
+ "github.com/rs/zerolog/log"
client "github.com/zapier/kubechecks/pkg/kubernetes"
+ "github.com/zapier/kubechecks/pkg/vcs/github_client"
+ "github.com/zapier/kubechecks/pkg/vcs/gitlab_client"
- "github.com/zapier/kubechecks/pkg"
- "github.com/zapier/kubechecks/pkg/app_watcher"
"github.com/zapier/kubechecks/pkg/appdir"
"github.com/zapier/kubechecks/pkg/argo_client"
"github.com/zapier/kubechecks/pkg/config"
@@ -17,35 +18,107 @@ import (
)
type Container struct {
- ApplicationWatcher *app_watcher.ApplicationWatcher
- ApplicationSetWatcher *app_watcher.ApplicationSetWatcher
- ArgoClient *argo_client.ArgoClient
+ ArgoClient *argo_client.ArgoClient
Config config.ServerConfig
RepoManager *git.RepoManager
VcsClient vcs.Client
- VcsToArgoMap VcsToArgoMap
+ VcsToArgoMap appdir.VcsToArgoMap
KubeClientSet client.Interface
}
-type VcsToArgoMap interface {
- AddApp(*v1alpha1.Application)
- AddAppSet(*v1alpha1.ApplicationSet)
- UpdateApp(old, new *v1alpha1.Application)
- UpdateAppSet(old *v1alpha1.ApplicationSet, new *v1alpha1.ApplicationSet)
- DeleteApp(*v1alpha1.Application)
- DeleteAppSet(app *v1alpha1.ApplicationSet)
- GetVcsRepos() []string
- GetAppsInRepo(string) *appdir.AppDirectory
- GetAppSetsInRepo(string) *appdir.AppSetDirectory
- GetMap() map[pkg.RepoURL]*appdir.AppDirectory
- WalkKustomizeApps(cloneURL string, fs fs.FS) *appdir.AppDirectory
-}
-
type ReposCache interface {
Clone(ctx context.Context, repoUrl string) (string, error)
CloneWithBranch(ctx context.Context, repoUrl, targetBranch string) (string, error)
}
+
+func New(ctx context.Context, cfg config.ServerConfig) (Container, error) {
+ var err error
+
+ var ctr = Container{
+ Config: cfg,
+ RepoManager: git.NewRepoManager(cfg),
+ }
+
+ // create vcs client
+ switch cfg.VcsType {
+ case "gitlab":
+ ctr.VcsClient, err = gitlab_client.CreateGitlabClient(cfg)
+ case "github":
+ ctr.VcsClient, err = github_client.CreateGithubClient(cfg)
+ default:
+ err = fmt.Errorf("unknown vcs-type: %q", cfg.VcsType)
+ }
+ if err != nil {
+ return ctr, errors.Wrap(err, "failed to create vcs client")
+ }
+ var kubeClient client.Interface
+
+ switch cfg.KubernetesType {
+ // TODO: expand with other cluster types
+ case client.ClusterTypeLOCAL:
+ kubeClient, err = client.New(&client.NewClientInput{
+ KubernetesConfigPath: cfg.KubernetesConfig,
+ ClusterType: cfg.KubernetesType,
+ })
+ if err != nil {
+ return ctr, errors.Wrap(err, "failed to create kube client")
+ }
+ case client.ClusterTypeEKS:
+ kubeClient, err = client.New(&client.NewClientInput{
+ KubernetesConfigPath: cfg.KubernetesConfig,
+ ClusterType: cfg.KubernetesType,
+ },
+ client.EKSClientOption(ctx, cfg.KubernetesClusterID),
+ )
+ if err != nil {
+ return ctr, errors.Wrap(err, "failed to create kube client")
+ }
+ }
+ ctr.KubeClientSet = kubeClient
+ // create argo client
+ if ctr.ArgoClient, err = argo_client.NewArgoClient(cfg, kubeClient); err != nil {
+ return ctr, errors.Wrap(err, "failed to create argo client")
+ }
+
+ // create vcs to argo map
+ vcsToArgoMap := appdir.NewVcsToArgoMap(ctr.VcsClient.Username())
+ ctr.VcsToArgoMap = vcsToArgoMap
+
+ if cfg.MonitorAllApplications {
+ if err = buildAppsMap(ctx, ctr.ArgoClient, ctr.VcsToArgoMap); err != nil {
+ log.Fatal().Err(err).Msg("failed to build apps map")
+ }
+
+ if err = buildAppSetsMap(ctx, ctr.ArgoClient, ctr.VcsToArgoMap); err != nil {
+ log.Fatal().Err(err).Msg("failed to build appsets map")
+ }
+ }
+
+ return ctr, nil
+}
+
+func buildAppsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result appdir.VcsToArgoMap) error {
+ apps, err := argoClient.GetApplications(ctx)
+ if err != nil {
+ return errors.Wrap(err, "failed to list applications")
+ }
+ for _, app := range apps.Items {
+ result.AddApp(&app)
+ }
+ return nil
+}
+
+func buildAppSetsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result appdir.VcsToArgoMap) error {
+ appSets, err := argoClient.GetApplicationSets(ctx)
+ if err != nil {
+ return errors.Wrap(err, "failed to list application sets")
+ }
+ for _, appSet := range appSets.Items {
+ result.AddAppSet(&appSet)
+ }
+ return nil
+} Feedback & Suggestions:
😼 Mergecat review of pkg/events/check_test.go@@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/require"
affectedappsmocks "github.com/zapier/kubechecks/mocks/affected_apps/mocks"
generatorsmocks "github.com/zapier/kubechecks/mocks/generator/mocks"
+ vcsmocks "github.com/zapier/kubechecks/mocks/vcs/mocks"
"github.com/zapier/kubechecks/pkg/affected_apps"
"github.com/zapier/kubechecks/pkg/checks"
"github.com/zapier/kubechecks/pkg/config"
@@ -65,12 +66,6 @@ func TestCleanupGetManifestsError(t *testing.T) {
}
}
-type mockVcsClient struct{}
-
-func (m mockVcsClient) Username() string {
- return "username"
-}
-
func TestCheckEventGetRepo(t *testing.T) {
cloneURL := "https://github.com/zapier/kubechecks.git"
canonical, err := canonicalize(cloneURL)
@@ -80,12 +75,16 @@ func TestCheckEventGetRepo(t *testing.T) {
ctx := context.TODO()
t.Run("empty branch name", func(t *testing.T) {
+ vcsClient := new(vcsmocks.MockClient)
+ vcsClient.EXPECT().Username().Return("username")
+
ce := CheckEvent{
- clonedRepos: make(map[string]*git.Repo),
+ clonedRepos: make(map[repoKey]*git.Repo),
repoManager: git.NewRepoManager(cfg),
+ ctr: container.Container{VcsClient: vcsClient},
}
- repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "")
+ repo, err := ce.getRepo(ctx, cloneURL, "")
require.NoError(t, err)
assert.Equal(t, "main", repo.BranchName)
assert.Len(t, ce.clonedRepos, 2)
@@ -94,12 +93,16 @@ func TestCheckEventGetRepo(t *testing.T) {
})
t.Run("branch is HEAD", func(t *testing.T) {
+ vcsClient := new(vcsmocks.MockClient)
+ vcsClient.EXPECT().Username().Return("username")
+
ce := CheckEvent{
- clonedRepos: make(map[string]*git.Repo),
+ clonedRepos: make(map[repoKey]*git.Repo),
repoManager: git.NewRepoManager(cfg),
+ ctr: container.Container{VcsClient: vcsClient},
}
- repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "HEAD")
+ repo, err := ce.getRepo(ctx, cloneURL, "HEAD")
require.NoError(t, err)
assert.Equal(t, "main", repo.BranchName)
assert.Len(t, ce.clonedRepos, 2)
@@ -108,12 +111,16 @@ func TestCheckEventGetRepo(t *testing.T) {
})
t.Run("branch is the same as HEAD", func(t *testing.T) {
+ vcsClient := new(vcsmocks.MockClient)
+ vcsClient.EXPECT().Username().Return("username")
+
ce := CheckEvent{
- clonedRepos: make(map[string]*git.Repo),
+ clonedRepos: make(map[repoKey]*git.Repo),
repoManager: git.NewRepoManager(cfg),
+ ctr: container.Container{VcsClient: vcsClient},
}
- repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "main")
+ repo, err := ce.getRepo(ctx, cloneURL, "main")
require.NoError(t, err)
assert.Equal(t, "main", repo.BranchName)
assert.Len(t, ce.clonedRepos, 2)
@@ -122,12 +129,16 @@ func TestCheckEventGetRepo(t *testing.T) {
})
t.Run("branch is not the same as HEAD", func(t *testing.T) {
+ vcsClient := new(vcsmocks.MockClient)
+ vcsClient.EXPECT().Username().Return("username")
+
ce := CheckEvent{
- clonedRepos: make(map[string]*git.Repo),
+ clonedRepos: make(map[repoKey]*git.Repo),
repoManager: git.NewRepoManager(cfg),
+ ctr: container.Container{VcsClient: vcsClient},
}
- repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "gh-pages")
+ repo, err := ce.getRepo(ctx, cloneURL, "gh-pages")
require.NoError(t, err)
assert.Equal(t, "gh-pages", repo.BranchName)
assert.Len(t, ce.clonedRepos, 1)
@@ -145,7 +156,7 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) {
ctr container.Container
repoManager repoManager
processors []checks.ProcessorEntry
- clonedRepos map[string]*git.Repo
+ clonedRepos map[repoKey]*git.Repo
addedAppsSet map[string]v1alpha1.Application
appsSent int32
appChannel chan *v1alpha1.Application
@@ -308,6 +319,7 @@ func MockGenerator(methodName string, returns []interface{}) generator.AppsGener
return mockClient
}
+
func MockInitMatcherFn() MatcherFn {
return func(ce *CheckEvent, repo *git.Repo) error {
return nil Feedback & Suggestions:
Overall, the changes improve the testability and maintainability of the code. Just ensure that all mock expectations are correctly set and that the new types are consistently used throughout the codebase. 👍 😼 Mergecat review of pkg/events/worker.go@@ -3,15 +3,18 @@ package events
import (
"context"
"fmt"
+ "runtime/debug"
"sync/atomic"
+ "github.com/ghodss/yaml"
+ "github.com/rs/zerolog/log"
+ "github.com/zapier/kubechecks/pkg/vcs"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/rs/zerolog"
"github.com/zapier/kubechecks/pkg"
- "github.com/zapier/kubechecks/pkg/argo_client"
"github.com/zapier/kubechecks/pkg/checks"
"github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/git"
@@ -20,14 +23,15 @@ import (
)
type worker struct {
- appChannel chan *v1alpha1.Application
- ctr container.Container
- logger zerolog.Logger
- processors []checks.ProcessorEntry
- vcsNote *msg.Message
+ appChannel chan *v1alpha1.Application
+ ctr container.Container
+ logger zerolog.Logger
+ processors []checks.ProcessorEntry
+ pullRequest vcs.PullRequest
+ vcsNote *msg.Message
done func()
- getRepo func(ctx context.Context, vcsClient hasUsername, cloneURL, branchName string) (*git.Repo, error)
+ getRepo func(ctx context.Context, cloneURL, branchName string) (*git.Repo, error)
queueApp, removeApp func(application v1alpha1.Application)
}
@@ -54,91 +58,86 @@ func (w *worker) processApp(ctx context.Context, app v1alpha1.Application) {
var (
err error
- appName = app.Name
- appSrc = app.Spec.Source
- appPath = appSrc.Path
- appRepoUrl = appSrc.RepoURL
+ appName = app.Name
- logger = w.logger.With().
- Str("app_name", appName).
- Str("app_path", appPath).
- Logger()
+ rootLogger = w.logger.With().
+ Str("app_name", appName).
+ Logger()
)
ctx, span := tracer.Start(ctx, "processApp", trace.WithAttributes(
attribute.String("app", appName),
- attribute.String("dir", appPath),
))
defer span.End()
atomic.AddInt32(&inFlight, 1)
defer atomic.AddInt32(&inFlight, -1)
- logger.Info().Msg("Processing app")
+ rootLogger.Info().Msg("Processing app")
// Build a new section for this app in the parent comment
w.vcsNote.AddNewApp(ctx, appName)
defer func() {
- if err := recover(); err != nil {
+ if r := recover(); r != nil {
desc := fmt.Sprintf("panic while checking %s", appName)
- w.logger.Error().Str("app", appName).Msgf("panic while running check")
+ w.logger.Error().Any("error", r).
+ Str("app", appName).Msgf("panic while running check")
+ println(string(debug.Stack()))
- telemetry.SetError(span, fmt.Errorf("%v", err), "panic while running check")
+ telemetry.SetError(span, fmt.Errorf("%v", r), "panic while running check")
result := msg.Result{
State: pkg.StatePanic,
Summary: desc,
- Details: fmt.Sprintf(errorCommentFormat, desc, err),
+ Details: fmt.Sprintf(errorCommentFormat, desc, r),
}
w.vcsNote.AddToAppMessage(ctx, appName, result)
}
}()
- repo, err := w.getRepo(ctx, w.ctr.VcsClient, appRepoUrl, appSrc.TargetRevision)
+ rootLogger.Debug().Msg("Getting manifests")
+ jsonManifests, err := w.ctr.ArgoClient.GetManifests(ctx, appName, app, w.pullRequest, w.getRepo)
if err != nil {
- logger.Error().Err(err).Msg("Unable to clone repository")
- w.vcsNote.AddToAppMessage(ctx, appName, msg.Result{
- State: pkg.StateError,
- Summary: "failed to clone repo",
- Details: fmt.Sprintf("Clone URL: `%s`\nTarget Revision: `%s`\n```\n%s\n```", appRepoUrl, appSrc.TargetRevision, err.Error()),
- })
- return
- }
- repoPath := repo.Directory
-
- logger.Debug().Str("repo_path", repoPath).Msg("Getting manifests")
- jsonManifests, err := w.ctr.ArgoClient.GetManifestsLocal(ctx, appName, repoPath, appPath, app)
- if err != nil {
- logger.Error().Err(err).Msg("Unable to get manifests")
+ rootLogger.Error().Err(err).Msg("Unable to get manifests")
w.vcsNote.AddToAppMessage(ctx, appName, msg.Result{
State: pkg.StateError,
Summary: "Unable to get manifests",
- Details: fmt.Sprintf("```\n%s\n```", cleanupGetManifestsError(err, repo.Directory)),
+ Details: fmt.Sprintf("```\n%s\n```", err),
})
return
}
// Argo diff logic wants unformatted manifests but everything else wants them as YAML, so we prepare both
- yamlManifests := argo_client.ConvertJsonToYamlManifests(jsonManifests)
- logger.Trace().Msgf("Manifests:\n%+v\n", yamlManifests)
+ yamlManifests := convertJsonToYamlManifests(jsonManifests)
+ rootLogger.Trace().Msgf("Manifests:\n%+v\n", yamlManifests)
k8sVersion, err := w.ctr.ArgoClient.GetKubernetesVersionByApplication(ctx, app)
if err != nil {
- logger.Error().Err(err).Msg("Error retrieving the Kubernetes version")
+ rootLogger.Error().Err(err).Msg("Error retrieving the Kubernetes version")
k8sVersion = w.ctr.Config.FallbackK8sVersion
} else {
k8sVersion = fmt.Sprintf("%s.0", k8sVersion)
- logger.Info().Msgf("Kubernetes version: %s", k8sVersion)
+ rootLogger.Info().Msgf("Kubernetes version: %s", k8sVersion)
}
- runner := newRunner(
- w.ctr, app, repo, appName, k8sVersion, jsonManifests, yamlManifests, logger, w.vcsNote,
- w.queueApp, w.removeApp,
- )
+ runner := newRunner(w.ctr, app, appName, k8sVersion, jsonManifests, yamlManifests, rootLogger, w.vcsNote, w.queueApp, w.removeApp)
for _, processor := range w.processors {
runner.Run(ctx, processor.Name, processor.Processor, processor.WorstState)
}
runner.Wait()
}
+
+func convertJsonToYamlManifests(jsonManifests []string) []string {
+ var manifests []string
+ for _, manifest := range jsonManifests {
+ ret, err := yaml.JSONToYAML([]byte(manifest))
+ if err != nil {
+ log.Warn().Err(err).Msg("Failed to format manifest")
+ continue
+ }
+ manifests = append(manifests, fmt.Sprintf("---\n%s", string(ret)))
+ }
+ return manifests
+} Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests.go package argo_client
import (
+ "bufio"
"context"
"fmt"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "regexp"
+ "strings"
"time"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
+ "github.com/argoproj/argo-cd/v2/pkg/apiclient/project"
"github.com/argoproj/argo-cd/v2/pkg/apiclient/settings"
- argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
repoapiclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient"
- "github.com/argoproj/argo-cd/v2/reposerver/repository"
- "github.com/argoproj/argo-cd/v2/util/git"
- "github.com/ghodss/yaml"
+ "github.com/argoproj/argo-cd/v2/util/argo"
+ "github.com/argoproj/argo-cd/v2/util/db"
+ argosettings "github.com/argoproj/argo-cd/v2/util/settings"
+ "github.com/argoproj/argo-cd/v2/util/tgzstream"
"github.com/pkg/errors"
"github.com/rs/zerolog/log"
- "k8s.io/apimachinery/pkg/api/resource"
-
- "github.com/zapier/kubechecks/telemetry"
+ "github.com/zapier/kubechecks/pkg"
+ "github.com/zapier/kubechecks/pkg/git"
+ "github.com/zapier/kubechecks/pkg/vcs"
)
-func (argo *ArgoClient) GetManifestsLocal(ctx context.Context, name, tempRepoDir, changedAppFilePath string, app argoappv1.Application) ([]string, error) {
- var err error
+type getRepo func(ctx context.Context, cloneURL string, branchName string) (*git.Repo, error)
- ctx, span := tracer.Start(ctx, "GetManifestsLocal")
+func (a *ArgoClient) GetManifests(ctx context.Context, name string, app v1alpha1.Application, pullRequest vcs.PullRequest, getRepo getRepo) ([]string, error) {
+ ctx, span := tracer.Start(ctx, "GetManifests")
defer span.End()
- log.Debug().Str("name", name).Msg("GetManifestsLocal")
+ log.Debug().Str("name", name).Msg("GetManifests")
start := time.Now()
defer func() {
duration := time.Since(start)
getManifestsDuration.WithLabelValues(name).Observe(duration.Seconds())
}()
- clusterCloser, clusterClient := argo.GetClusterClient()
- defer clusterCloser.Close()
+ contentRefs, refs := preprocessSources(&app, pullRequest)
- settingsCloser, settingsClient := argo.GetSettingsClient()
- defer settingsCloser.Close()
+ var manifests []string
+ for _, source := range contentRefs {
+ moreManifests, err := a.generateManifests(ctx, app, source, refs, pullRequest, getRepo)
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to generate manifests")
+ }
+ manifests = append(manifests, moreManifests...)
+ }
+
+ getManifestsSuccess.WithLabelValues(name).Inc()
+ return manifests, nil
+}
+
+// preprocessSources splits the content sources from the ref sources, and transforms source refs that point at the pull
+// request's base into refs that point at the pull request's head. This is necessary to generate manifests based on what
+// the world will look like _after_ the branch gets merged in.
+func preprocessSources(app *v1alpha1.Application, pullRequest vcs.PullRequest) ([]v1alpha1.ApplicationSource, []v1alpha1.ApplicationSource) {
+ if !app.Spec.HasMultipleSources() {
+ return []v1alpha1.ApplicationSource{app.Spec.GetSource()}, nil
+ }
+
+ // collect all ref sources, map by name
+ var contentSources []v1alpha1.ApplicationSource
+ var refSources []v1alpha1.ApplicationSource
+
+ for _, source := range app.Spec.Sources {
+ if source.Ref == "" {
+ contentSources = append(contentSources, source)
+ continue
+ }
+
+ /*
+ This is to make sure that the respository server understands where to pull the values.yaml file from.
+
+ Or put differently:
+
+ | PR Repo | PR Base | PR Target | Ref Repo | Ref Target | |
+ | --------- | ----------- | --------- | --------- | ---------- | ----------------------------------------------------------------------------------------------- |
+ | repo1.git | new-feature | main | repo1.git | main | need to change main to new-feature for preview, as the base will become the target after merge. |
+ | repo1.git | new-feature | main | repo2.git | main | no change, ref source refers to a different repository unaffected by the pull request |
+ | repo1.git | new-feature | main | repo1.git | staging | no change, ref source refers to a different branch than the pull request |
+ */
+ if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) {
+ if source.TargetRevision == pullRequest.BaseRef {
+ source.TargetRevision = pullRequest.HeadRef
+ }
+ }
+
+ refSources = append(refSources, source)
+ }
+
+ return contentSources, refSources
+}
+
+// generateManifests generates an Application along with all of its files, and sends it to the ArgoCD
+// Repository service to be transformed into raw kubernetes manifests. This allows us to take advantage of server
+// configuration and credentials.
+func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Application, source v1alpha1.ApplicationSource, refs []v1alpha1.ApplicationSource, pullRequest vcs.PullRequest, getRepo func(ctx context.Context, cloneURL string, branchName string) (*git.Repo, error)) ([]string, error) {
+ // The GenerateManifestWithFiles has some non-obvious rules due to assumptions that it makes:
+ // 1. first source must be a non-ref source
+ // 2. there must be one and only one non-ref source
+ // 3. ref sources that match the pull requests' repo and target branch need to have their target branch swapped to the head branch of the pull request
+
+ clusterCloser, clusterClient := a.GetClusterClient()
+ defer clusterCloser.Close()
- 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()
+ getManifestsFailed.WithLabelValues(app.Name).Inc()
return nil, errors.Wrap(err, "failed to get cluster")
}
+ settingsCloser, settingsClient := a.GetSettingsClient()
+ defer settingsCloser.Close()
+
+ log.Info().Msg("get settings")
argoSettings, err := settingsClient.Get(ctx, &settings.SettingsQuery{})
if err != nil {
- telemetry.SetError(span, err, "Argo Get Settings")
- getManifestsFailed.WithLabelValues(name).Inc()
+ getManifestsFailed.WithLabelValues(app.Name).Inc()
return nil, errors.Wrap(err, "failed to get settings")
}
- log.Debug().Str("name", name).Msg("generating diff for application...")
- res, err := argo.generateManifests(ctx, fmt.Sprintf("%s/%s", tempRepoDir, changedAppFilePath), tempRepoDir, app, argoSettings, cluster)
+ settingsMgr := argosettings.NewSettingsManager(ctx, a.k8s, a.namespace)
+ argoDB := db.NewDB(a.namespace, settingsMgr, a.k8s)
+
+ repoTarget := source.TargetRevision
+ if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) && areSameTargetRef(source.TargetRevision, pullRequest.BaseRef) {
+ repoTarget = pullRequest.HeadRef
+ }
+
+ log.Info().Msg("get repo")
+ repo, err := getRepo(ctx, source.RepoURL, repoTarget)
if err != nil {
- telemetry.SetError(span, err, "Generate Manifests")
- return nil, errors.Wrap(err, "failed to generate manifests")
+ return nil, errors.Wrap(err, "failed to get repo")
}
- if res.Manifests == nil {
- return nil, nil
+ var packageDir string
+ if a.sendFullRepository {
+ log.Info().Msg("sending full repository")
+ packageDir = repo.Directory
+ } else {
+ log.Info().Msg("packaging app")
+ packageDir, err = packageApp(ctx, source, refs, repo, getRepo)
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to package application")
+ }
+ }
+
+ log.Info().Msg("compressing files")
+ f, filesWritten, checksum, err := tgzstream.CompressFiles(packageDir, []string{"*"}, []string{".git"})
+ if err != nil {
+ return nil, fmt.Errorf("failed to compress files: %w", err)
+ }
+ log.Info().Msgf("%d files compressed", filesWritten)
+ //if filesWritten == 0 {
+ // return nil, fmt.Errorf("no files to send")
+ //}
+
+ closer, projectClient, err := a.client.NewProjectClient()
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to get project client")
+ }
+ defer closer.Close()
+
+ proj, err := projectClient.Get(ctx, &project.ProjectQuery{Name: app.Spec.Project})
+ if err != nil {
+ return nil, fmt.Errorf("error getting app project: %w", err)
+ }
+
+ helmRepos, err := argoDB.ListHelmRepositories(ctx)
+ if err != nil {
+ return nil, fmt.Errorf("error listing helm repositories: %w", err)
+ }
+ permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos)
+ if err != nil {
+ return nil, fmt.Errorf("error retrieving permitted repos: %w", err)
+ }
+ helmRepositoryCredentials, err := argoDB.GetAllHelmRepositoryCredentials(ctx)
+ if err != nil {
+ return nil, fmt.Errorf("error getting helm repository credentials: %w", err)
+ }
+ helmOptions, err := settingsMgr.GetHelmSettings()
+ if err != nil {
+ return nil, fmt.Errorf("error getting helm settings: %w", err)
+ }
+ permittedHelmCredentials, err := argo.GetPermittedReposCredentials(proj, helmRepositoryCredentials)
+ if err != nil {
+ return nil, fmt.Errorf("error getting permitted repos credentials: %w", err)
+ }
+ enabledSourceTypes, err := settingsMgr.GetEnabledSourceTypes()
+ if err != nil {
+ return nil, fmt.Errorf("error getting settings enabled source types: %w", err)
+ }
+
+ refSources, err := argo.GetRefSources(context.Background(), app.Spec.Sources, app.Spec.Project, argoDB.GetRepository, []string{}, false)
+ if err != nil {
+ return nil, fmt.Errorf("failed to get ref sources: %w", err)
+ }
+
+ app.Spec.Sources = append([]v1alpha1.ApplicationSource{source}, refs...)
+
+ q := repoapiclient.ManifestRequest{
+ Repo: &v1alpha1.Repository{Repo: source.RepoURL},
+ Revision: source.TargetRevision,
+ AppLabelKey: argoSettings.AppLabelKey,
+ AppName: app.Name,
+ Namespace: app.Spec.Destination.Namespace,
+ ApplicationSource: &source,
+ Repos: permittedHelmRepos,
+ KustomizeOptions: argoSettings.KustomizeOptions,
+ KubeVersion: cluster.Info.ServerVersion,
+ ApiVersions: cluster.Info.APIVersions,
+ HelmRepoCreds: permittedHelmCredentials,
+ HelmOptions: helmOptions,
+ TrackingMethod: argoSettings.TrackingMethod,
+ EnabledSourceTypes: enabledSourceTypes,
+ ProjectName: proj.Name,
+ ProjectSourceRepos: proj.Spec.SourceRepos,
+ HasMultipleSources: app.Spec.HasMultipleSources(),
+ RefSources: refSources,
+ }
+
+ log.Info().Msg("generating manifest with files")
+ stream, err := a.repoClient.GenerateManifestWithFiles(ctx)
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to get manifests with files")
}
- getManifestsSuccess.WithLabelValues(name).Inc()
- return res.Manifests, nil
-}
-func (argo *ArgoClient) generateManifests(
- ctx context.Context, appPath, tempRepoDir string, app argoappv1.Application, argoSettings *settings.Settings, cluster *argoappv1.Cluster,
-) (*repoapiclient.ManifestResponse, error) {
- argo.manifestsLock.Lock()
- defer argo.manifestsLock.Unlock()
-
- source := app.Spec.GetSource()
-
- return repository.GenerateManifests(
- ctx,
- appPath,
- tempRepoDir,
- source.TargetRevision,
- &repoapiclient.ManifestRequest{
- Repo: &argoappv1.Repository{Repo: source.RepoURL},
- AppLabelKey: argoSettings.AppLabelKey,
- AppName: app.Name,
- Namespace: app.Spec.Destination.Namespace,
- ApplicationSource: &source,
- KustomizeOptions: argoSettings.KustomizeOptions,
- KubeVersion: cluster.Info.ServerVersion,
- ApiVersions: cluster.Info.APIVersions,
- TrackingMethod: argoSettings.TrackingMethod,
+ log.Info().Msg("sending request")
+ if err := stream.Send(&repoapiclient.ManifestRequestWithFiles{
+ Part: &repoapiclient.ManifestRequestWithFiles_Request{
+ Request: &q,
},
- true,
- new(git.NoopCredsStore),
- resource.MustParse("0"),
- nil,
- )
+ }); err != nil {
+ return nil, errors.Wrap(err, "failed to send request")
+ }
+
+ log.Info().Msg("sending metadata")
+ if err := stream.Send(&repoapiclient.ManifestRequestWithFiles{
+ Part: &repoapiclient.ManifestRequestWithFiles_Metadata{
+ Metadata: &repoapiclient.ManifestFileMetadata{
+ Checksum: checksum,
+ },
+ },
+ }); err != nil {
+ return nil, errors.Wrap(err, "failed to send metadata")
+ }
+
+ log.Info().Msg("sending file")
+ err = sendFile(ctx, stream, f)
+ if err != nil {
+ return nil, fmt.Errorf("failed to send manifest stream file: %w", err)
+ }
+
+ log.Info().Msg("receiving repsonse")
+ response, err := stream.CloseAndRecv()
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to get response")
+ }
+
+ log.Info().Msg("done!")
+ return response.Manifests, nil
}
-func ConvertJsonToYamlManifests(jsonManifests []string) []string {
- var manifests []string
- for _, manifest := range jsonManifests {
- ret, err := yaml.JSONToYAML([]byte(manifest))
+func copyFile(srcpath, dstpath string) error {
+ dstdir := filepath.Dir(dstpath)
+ if err := os.MkdirAll(dstdir, 0o777); err != nil {
+ return errors.Wrap(err, "failed to make directories")
+ }
+
+ r, err := os.Open(srcpath)
+ if err != nil {
+ return err
+ }
+ defer r.Close() // ignore error: file was opened read-only.
+
+ w, err := os.Create(dstpath)
+ if err != nil {
+ return err
+ }
+
+ defer func() {
+ // Report the error, if any, from Close, but do so
+ // only if there isn't already an outgoing error.
+ if c := w.Close(); err == nil {
+ err = c
+ }
+ }()
+
+ _, err = io.Copy(w, r)
+ return err
+}
+
+func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v1alpha1.ApplicationSource, repo *git.Repo, getRepo getRepo) (string, error) {
+ tempDir, err := os.MkdirTemp("", "package-*")
+ if err != nil {
+ return "", errors.Wrap(err, "failed to make temp dir")
+ }
+
+ tempAppDir := filepath.Join(tempDir, source.Path)
+ appPath := filepath.Join(repo.Directory, source.Path)
+
+ // copy app files to the temp dir
+ if err = filepath.Walk(appPath, func(path string, info fs.FileInfo, err error) error {
if err != nil {
- log.Warn().Err(err).Msg("Failed to format manifest")
- continue
+ return err
+ }
+
+ if info.IsDir() {
+ return nil
+ }
+
+ relPath, err := filepath.Rel(appPath, path)
+ if err != nil {
+ return errors.Wrapf(err, "failed to calculate rel between %q and %q", appPath, path)
+ }
+ src :=
</details>
---
## Dependency Review
<details><summary>Click to read mergecats review!</summary>
No suggestions found
</details> |
appreciate your work on this! |
this also removes the ability to reference a schemas dir relative to the root repo, but I'm hoping it's unused, as it's complicated to reason about or manage. a central schemas repo makes more sense.
We are now using argocd-repo-server to generate manifests.