feat: application set handler #241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. added appset watcher 2. change: cluster role (applicationset permisssion), ability to read/get/list secrets 3. change: updated the helm chart version 4. change: tiltfile for appset, Tile will no longer install appset yaml to the local k8s, instead, copies the template appsets yaml with replaced repo_url. Terraform will copy the file to the appropriate repo. 5. new: kubernetes client for EKS. 6. new: in EKS Client, STS authentication is automated with a custom http transport (to auto refresh token) 7. new: extended the cobra flag, and serverConfig to handle kubernetes-type (e.g. eks, local), kubernetes-clusterid, kubernetes-cluster-region 8. change: moved appwatcher's kubeclient init to container and instead pass the rest.Config as parameter to appwatcher. 9. new: forked generator code from argo-cd (excluding pull_request generator as it is not compatible with go-gitlab v0.105)
Signed-off-by: James Hong <greyeye@gmail.com>
|
Temporary image deleted. |
Closed
1. add new helm chart to help install role/rolebinding to grant access to the kubernetes cluster remotely.
|
would you be kind enough to review the PR?
|
MeNsaaH
requested changes
Jul 15, 2024
localdev/terraform/modules/vcs_files/base_files/appsets/echo-server/.gitkeep
Outdated
Show resolved
Hide resolved
pkg/app_watcher/appset_watcher.go
Outdated
There was a problem hiding this comment.
sure will add one tomorrow
There was a problem hiding this comment.
this has been added
1. remove .gitrepo, tiltfile will use mkdir -p to create the target folder. 2. app_directory, FindAppsBasedOnChangeList() will not longer use git.repo 3, remove stack from debug msgs in check.go, as it is not a correct use. 4. extended VcsToArgoMap interface
djeebus
reviewed
Jul 16, 2024
1. change kube client type from localhost to local 2. appset_directory unit test
djeebus
reviewed
Jul 25, 2024
pkg/kubernetes/api_eks_client.go
Outdated
| // - clusterID: the name of the EKS cluster. | ||
| // | ||
| // - region: the AWS region where the EKS cluster is located. (optional) If not provided, the default region will be used. | ||
| func EKSClientOption(ctx context.Context, clusterID, region string) NewClientOption { |
There was a problem hiding this comment.
I don't think you need region, since AWS_REGION will let users override this themselves directly.
There was a problem hiding this comment.
@djeebus i've removed the region parameter.
|
I saw your successful test; nice work! I've also confirmed backwards compatibility. 👍 |
djeebus
approved these changes
Jul 26, 2024
Signed-off-by: James Hong <greyeye@gmail.com>
1. update the chart unit test with the correct test
djeebus
reviewed
Jul 29, 2024
charts/kubechecks-rbac/values.yaml
Outdated
Comment on lines
1
to
3
| ClusterRoleName: "kubechecks-remote-role" | ||
| ClusterRoleBindingName: "kubechecks-remote-role-binding" | ||
| ClusterRoleBindingGroup: "kubechecks-remote-group" |
There was a problem hiding this comment.
these should start with a lowercase letter, for consistency
There was a problem hiding this comment.
good point, changed to lowercase
djeebus
reviewed
Jul 29, 2024
pkg/argo_client/applications.go
Outdated
| telemetry.SetError(span, err, "Argo List All Applications error") | ||
| return nil, errors.Wrap(err, "failed to applications") | ||
| telemetry.SetError(span, err, "Argo List All Application Sets error") | ||
| return nil, errors.Wrap(err, "failed to application sets") |
There was a problem hiding this comment.
probably should be "failed to list application sets"
There was a problem hiding this comment.
changed it to failed to list application sets.
MeNsaaH
approved these changes
Jul 30, 2024
Mergecat's ReviewClick to read mergecats review!😼 Mergecat review of localdev/terraform/modules/vcs_files/base_files/apps/echo-server/in-cluster/values.yaml@@ -1,2 +1,2 @@
echo-server:
- replicaCount: 2
\ No newline at end of file
+ replicaCount: 1
\ No newline at end of fileFeedback & Suggestions:
😼 Mergecat review of charts/kubechecks/Chart.yaml@@ -1,7 +1,7 @@
apiVersion: v2
name: kubechecks
description: A Helm chart for kubechecks
-version: 0.4.4
+version: 0.4.5
type: application
maintainers:
- name: zapierFeedback & Suggestions: The diff correctly updates the version number from 0.4.4 to 0.4.5. There are no issues with this change. 👍 😼 Mergecat review of .mockery.yaml@@ -5,3 +5,11 @@ packages:
# place your package-specific config here
config:
all: true
+ github.com/zapier/kubechecks/pkg/generator:
+ # place your package-specific config here
+ config:
+ all: true
+ github.com/zapier/kubechecks/pkg/affected_apps:
+ # place your package-specific config here
+ config:
+ all: trueFeedback & Suggestions: It appears that the new diff is duplicating entries that already exist in the original code. This redundancy can lead to confusion and potential maintenance issues. Please remove the duplicated entries to keep the configuration clean and concise. 😼 Mergecat review of localdev/.gitignore@@ -1,4 +1,6 @@
terraform.tfstate*
.terraform.lock.hcl
.terraform/**
-.terraform.tfstate.lock.info
\ No newline at end of file
+.terraform.tfstate.lock.info
+/terraform/modules/vcs_files/base_files/appsets/httpdump/httpdump.yaml
+/terraform/modules/vcs_files/base_files/appsets/echo-server/echo-server.yamlFeedback & Suggestions:
Suggested Diff: @@ -1,4 +1,5 @@
terraform.tfstate*
.terraform.lock.hcl
.terraform/**
.terraform.tfstate.lock.info
+/terraform/modules/vcs_files/base_files/appsets/httpdump/httpdump.yaml
+/terraform/modules/vcs_files/base_files/appsets/echo-server/echo-server.yaml😼 Mergecat review of charts/kubechecks/values.yaml@@ -6,6 +6,7 @@ configMap:
env: {}
# KUBECHECKS_ARGOCD_API_INSECURE: "false"
# KUBECHECKS_ARGOCD_API_PATH_PREFIX: /
+ # KUBECHECKS_ARGOCD_API_NAMESPACE: argocd
# KUBECHECKS_ARGOCD_WEBHOOK_URL: https://argocd.<domain.com>/api/webhook
# KUBECHECKS_FALLBACK_K8S_VERSION: "1.22.0"
# KUBECHECKS_LOG_LEVEL: debugFeedback & Suggestions:
😼 Mergecat review of pkg/affected_apps/argocd_matcher_test.go@@ -34,7 +34,7 @@ func TestFindAffectedAppsWithNilAppsDirectory(t *testing.T) {
)
matcher := ArgocdMatcher{}
- items, err := matcher.AffectedApps(ctx, changeList, "main")
+ items, err := matcher.AffectedApps(ctx, changeList, "main", nil)
// verify results
require.NoError(t, err)Feedback & Suggestions:
😼 Mergecat review of localdev/argocd/kustomization.yaml@@ -6,7 +6,7 @@ namespace: kubechecks
images:
- name: quay.io/argoproj/argocd
newName: quay.io/argoproj/argocd
- newTag: v2.6.12
+ newTag: v2.11.3
resources:
- argocd-initial-admin-secret.yamlFeedback & Suggestions:
😼 Mergecat review of charts/kubechecks/templates/clusterrole.yaml@@ -4,5 +4,8 @@ metadata:
name: {{ include "kubechecks.fullname" . }}
rules:
- apiGroups: ['argoproj.io']
- resources: ['applications', 'appprojects', 'services']
+ resources: ['applications', 'appprojects', 'applicationsets', 'services']
+ verbs: ['get', 'list', 'watch']
+ - apiGroups: [''] # The core API group, which is indicated by an empty string
+ resources: ['secrets']
verbs: ['get', 'list', 'watch']Feedback & Suggestions:
Suggested Diff:@@ -4,5 +4,6 @@ metadata:
name: {{ include "kubechecks.fullname" . }}
rules:
- apiGroups: ['argoproj.io']
- resources: ['applications', 'appprojects', 'services']
+ resources: ['applications', 'appprojects', 'applicationsets', 'services']
- apiGroups: [''] # The core API group, which is indicated by an empty string
resources: ['secrets']
verbs: ['get', 'list', 'watch']😼 Mergecat review of pkg/events/worker.go@@ -5,11 +5,11 @@ import (
"fmt"
"sync/atomic"
- "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
- "github.com/rs/zerolog"
"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"Feedback & Suggestions:
import (
"context"
"fmt"
"sync/atomic"
"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"
"github.com/zapier/kubechecks/pkg/msg"
"github.com/zapier/kubechecks/telemetry"
)This approach makes it easier to distinguish between different types of imports. 🛠️ 😼 Mergecat review of localdev/test_apps/Tiltfile@@ -13,11 +13,12 @@ def install_test_apps(cfg):
projectUrl=str(read_file(vcsPath, "")).strip('\n')
print("Remote Project URL: " + projectUrl)
- for app in ["echo-server", "httpbin"]:
+ for app in ["echo-server", "httpbin", "app-root"]:
print("Creating Test App: " + app)
# read the application YAML and patch the repoURL
objects = read_yaml_stream("localdev/test_apps/{}.yaml".format(app))
+
for o in objects:
o['metadata']['namespace'] = "kubechecks"
o['spec']['source']['repoURL'] = projectUrlFeedback & Suggestions:
Suggested diff: @@ -13,11 +13,12 @@ def install_test_apps(cfg):
projectUrl=str(read_file(vcsPath, "")).strip('\n')
print("Remote Project URL: " + projectUrl)
- for app in ["echo-server", "httpbin"]:
+ for app in ["echo-server", "httpbin", "app-root"]:
print("Creating Test App: " + app)
# read the application YAML and patch the repoURL
objects = read_yaml_stream("localdev/test_apps/{}.yaml".format(app))
-
+
for o in objects:
o['metadata']['namespace'] = "kubechecks"
o['spec']['source']['repoURL'] = projectUrl😼 Mergecat review of pkg/argo_client/applications.go@@ -132,8 +132,8 @@ func (argo *ArgoClient) GetApplicationSets(ctx context.Context) (*v1alpha1.Appli
resp, err := appClient.List(ctx, new(applicationset.ApplicationSetListQuery))
if err != nil {
- telemetry.SetError(span, err, "Argo List All Applications error")
- return nil, errors.Wrap(err, "failed to applications")
+ telemetry.SetError(span, err, "Argo List All Application Sets error")
+ return nil, errors.Wrap(err, "failed to list application sets")
}
return resp, nil
}Feedback & Suggestions:
Overall, the changes are appropriate and improve the clarity of the error messages. No further improvements are necessary for this diff. 👍 😼 Mergecat review of pkg/affected_apps/matcher.go@@ -5,11 +5,12 @@ import (
"path"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ "github.com/zapier/kubechecks/pkg/git"
)
type AffectedItems struct {
Applications []v1alpha1.Application
- ApplicationSets []ApplicationSet
+ ApplicationSets []v1alpha1.ApplicationSet
}
func (ai AffectedItems) Union(other AffectedItems) AffectedItems {
@@ -48,7 +49,7 @@ type ApplicationSet struct {
}
type Matcher interface {
- AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error)
+ AffectedApps(ctx context.Context, changeList []string, targetBranch string, repo *git.Repo) (AffectedItems, error)
}
// modifiedDirs filters a list of changed files down to a listFeedback & Suggestions:
By addressing these points, the code will be cleaner, more consistent, and less prone to errors. 🛠️ 😼 Mergecat review of pkg/config/config.go@@ -17,12 +17,14 @@ import (
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"`
- KubernetesConfig string `mapstructure:"kubernetes-config"`
+ 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"`
+ 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 localdev/test_appsets/httpdump.yaml@@ -5,6 +5,7 @@ metadata:
namespace: kubechecks
spec:
generators:
+ # this is a simple list generator
- list:
elements:
- name: a
@@ -16,7 +17,7 @@ spec:
finalizers:
- resources-finalizer.argocd.argoproj.io
name: "in-cluster-{{ name }}-httpdump"
- namespace: argocd
+ namespace: kubechecks
labels:
argocd.argoproj.io/application-set-name: "httpdump"
spec:
@@ -25,11 +26,12 @@ spec:
server: '{{ url }}'
project: default
source:
- repoURL: ${REPO_URL}
+ repoURL: REPO_URL
targetRevision: HEAD
path: 'apps/httpdump/overlays/{{ name }}/'
syncPolicy:
automated:
prune: true
syncOptions:
- - CreateNamespace=true
\ No newline at end of file
+ - CreateNamespace=true
+ sources: []Feedback & Suggestions:
😼 Mergecat review of pkg/appdir/app_directory.go@@ -58,13 +58,18 @@ func (d *AppDirectory) ProcessApp(app v1alpha1.Application) {
}
}
+// FindAppsBasedOnChangeList receives a list of modified file paths and
+// returns the list of applications that are affected by the changes.
+//
+// changeList: a slice of strings representing the paths of modified files.
+// targetBranch: the branch name to compare against the target revision of the applications.
+// e.g. changeList = ["path/to/file1", "path/to/file2"]
func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBranch string) []v1alpha1.Application {
log.Debug().Msgf("checking %d changes", len(changeList))
appsSet := make(map[string]struct{})
for _, changePath := range changeList {
log.Debug().Msgf("change: %s", changePath)
-
for dir, appNames := range d.appDirs {
if strings.HasPrefix(changePath, dir) {
log.Debug().Msg("dir match!")
@@ -146,10 +151,15 @@ func (d *AppDirectory) GetApps(filter func(stub v1alpha1.Application) bool) []v1
}
func (d *AppDirectory) AddApp(app v1alpha1.Application) {
+ if _, exists := d.appsMap[app.Name]; exists {
+ log.Debug().Msgf("app %s already exists", app.Name)
+ return
+ }
log.Debug().
Str("appName", app.Name).
Str("cluster-name", app.Spec.Destination.Name).
Str("cluster-server", app.Spec.Destination.Server).
+ Str("source", getSourcePath(app)).
Msg("add app")
d.appsMap[app.Name] = app
d.AddDir(app.Name, getSourcePath(app))Feedback & Suggestions:
😼 Mergecat review of pkg/affected_apps/multi_matcher.go@@ -4,6 +4,7 @@ import (
"context"
"github.com/pkg/errors"
+ "github.com/zapier/kubechecks/pkg/git"
)
func NewMultiMatcher(matchers ...Matcher) Matcher {
@@ -14,11 +15,11 @@ type MultiMatcher struct {
matchers []Matcher
}
-func (m MultiMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
+func (m MultiMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string, repo *git.Repo) (AffectedItems, error) {
var total AffectedItems
for index, matcher := range m.matchers {
- items, err := matcher.AffectedApps(ctx, changeList, targetBranch)
+ items, err := matcher.AffectedApps(ctx, changeList, targetBranch, repo)
if err != nil {
return total, errors.Wrapf(err, "failed to find items in matcher #%d", index)
}Feedback & Suggestions:
Here is the improved diff: @@ -4,6 +4,7 @@ import (
"context"
"github.com/pkg/errors"
)
func NewMultiMatcher(matchers ...Matcher) Matcher {
@@ -14,11 +15,11 @@ type MultiMatcher struct {
matchers []Matcher
}
-func (m MultiMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
+func (m MultiMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string, repo *git.Repo) (AffectedItems, error) {
var total AffectedItems
for index, matcher := range m.matchers {
- items, err := matcher.AffectedApps(ctx, changeList, targetBranch)
+ items, err := matcher.AffectedApps(ctx, changeList, targetBranch, repo)
if err != nil {
return total, errors.Wrapf(err, "failed to find items in matcher #%d", index)
}😼 Mergecat review of pkg/affected_apps/config_matcher.go@@ -9,6 +9,8 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/pkg/errors"
"github.com/rs/zerolog/log"
+ "github.com/zapier/kubechecks/pkg/git"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/repo_config"
@@ -28,9 +30,9 @@ func NewConfigMatcher(cfg *repo_config.Config, ctr container.Container) *ConfigM
return &ConfigMatcher{cfg: cfg, argoClient: ctr.ArgoClient}
}
-func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
+func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, _ string, _ *git.Repo) (AffectedItems, error) {
triggeredAppsMap := make(map[string]string)
- var appSetList []ApplicationSet
+ var appSetList []v1alpha1.ApplicationSet
triggeredApps, triggeredAppsets, err := b.triggeredApps(ctx, changeList)
if err != nil {
@@ -42,7 +44,11 @@ func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, t
}
for _, appset := range triggeredAppsets {
- appSetList = append(appSetList, ApplicationSet{appset.Name})
+ appSetList = append(appSetList, v1alpha1.ApplicationSet{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: appset.Name,
+ },
+ })
}
allArgoApps, err := b.argoClient.GetApplications(ctx)Feedback & Suggestions:
Overall, the changes look good and improve the type consistency in the code. 🛠️ 😼 Mergecat review of docs/architecture.md@@ -36,3 +36,10 @@ By abstracting the PR/MR in this way, `kubechecks` remains VCS provider agnostic
{: style="height:350px;display:block;margin:0 auto;}
The final piece of the puzzle is the `CheckEvent`; an internal structure that takes a `Client` and a `Repo` and begins running all configured checks. A `CheckEvent` first determines what applications within the repository have been affected by the PR/MR, and begins concurrently running the check suite against each affected application to generate a report for that app. As each application updates its report, the `CheckEvent` compiles all reports together and instructs the `Client` to update the PR/MR with a comment detailing the current progress; resulting in one comment per run of `kubechecks` with the latest information about that particular run. Whenever a new run of `kubechecks` is initiated, all previous comments are deleted to reduce clutter.
+
+### Event Flow Diagram
+
+{: style="height:350px;display:block;margin:0 auto;}
+
+This diagram illustrates the flow of events from the initial webhook trigger to the final report generation and comment update process.
+Feedback & Suggestions:
Overall, the diff is well-constructed and integrates seamlessly with the existing document. No further improvements are necessary. Great work! 🌟 😼 Mergecat review of pkg/app_watcher/app_watcher.go@@ -2,18 +2,18 @@ package app_watcher
import (
"context"
+ "fmt"
"reflect"
"strings"
"time"
- appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
- "github.com/rs/zerolog/log"
- "k8s.io/client-go/tools/clientcmd"
-
appv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
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"
"k8s.io/apimachinery/pkg/util/runtime"
+ "k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
"github.com/zapier/kubechecks/pkg/appdir"
@@ -29,18 +29,17 @@ type ApplicationWatcher struct {
vcsToArgoMap appdir.VcsToArgoMap
}
-// NewApplicationWatcher creates new instance of ApplicationWatcher.
-func NewApplicationWatcher(vcsToArgoMap appdir.VcsToArgoMap, cfg config.ServerConfig) (*ApplicationWatcher, error) {
- // this assumes kubechecks is running inside the cluster
- kubeCfg, err := clientcmd.BuildConfigFromFlags("", cfg.KubernetesConfig)
- if err != nil {
- log.Fatal().Msgf("Error building kubeconfig: %s", err.Error())
+// NewApplicationWatcher creates a new instance of ApplicationWatcher.
+//
+// - 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 {
+ return nil, fmt.Errorf("kubeCfg cannot be nil")
}
-
- appClient := appclientset.NewForConfigOrDie(kubeCfg)
-
ctrl := ApplicationWatcher{
- applicationClientset: appClient,
+ applicationClientset: appclientset.NewForConfigOrDie(kubeCfg),
vcsToArgoMap: vcsToArgoMap,
}
Feedback & Suggestions:
😼 Mergecat review of Tiltfile@@ -5,6 +5,7 @@ load('ext://tests/golang', 'test_go')
load('ext://namespace', 'namespace_create')
load('ext://uibutton', 'cmd_button')
load('ext://helm_resource', 'helm_resource')
+load('ext://local_output', 'local_output')
load('./.tilt/terraform/Tiltfile', 'local_terraform_resource')
load('./.tilt/utils/Tiltfile', 'check_env_set')
@@ -144,10 +145,6 @@ test_go(
)
-# get the git commit ref
-def get_git_head():
- result = local('git rev-parse --short HEAD')
- return result
# read .tool-versions file and return a dictionary of tools and their versions
def parse_tool_versions(fn):
@@ -174,7 +171,9 @@ def parse_tool_versions(fn):
return tools
tool_versions = parse_tool_versions(".tool-versions")
-git_commit = str(get_git_head()).strip()
+
+# get the git commit ref
+git_commit = local_output('git rev-parse --short HEAD')
earthly_build(
context='.',
@@ -260,8 +259,8 @@ k8s_resource(
load("localdev/test_apps/Tiltfile", "install_test_apps")
install_test_apps(cfg)
-load("localdev/test_appsets/Tiltfile", "install_test_appsets")
-install_test_appsets(cfg)
+load("localdev/test_appsets/Tiltfile", "copy_test_appsets")
+copy_test_appsets(cfg)
force_argocd_cleanup_on_tilt_down()Feedback & Suggestions:
😼 Mergecat review of localdev/test_appsets/Tiltfile@@ -3,28 +3,20 @@
# Test ArgoCD Applications
# /////////////////////////////////////////////////////////////////////////////
-def install_test_appsets(cfg):
+def copy_test_appsets(cfg):
# Load the terraform url we output, default to gitlab if cant find a vcs-type variable
vcsPath = "./localdev/terraform/{}/project.url".format(cfg.get('vcs-type', 'gitlab'))
print("Path to url: " + vcsPath)
projectUrl=str(read_file(vcsPath, "")).strip('\n')
print("Remote Project URL: " + projectUrl)
- k8s_kind('ApplicationSets', api_version="apiextensions.k8s.io/v1")
-
if projectUrl != "":
- for appset in ["httpdump"]:
- print("Creating Test ApplicationSet: " + appset)
+ for appset in ["httpdump","echo-server"]:
+ source_file = "./localdev/test_appsets/{}.yaml".format(appset)
+ dest_file = "./localdev/terraform/modules/vcs_files/base_files/appsets/{}/{}.yaml".format(appset,appset)
- # read the application YAML and patch the repoURL
- objects = read_yaml_stream("localdev/test_appsets/{}.yaml".format(appset))
- for o in objects:
- o['spec']['template']['spec']['source']['repoURL'] = projectUrl
- k8s_yaml(encode_yaml_stream(objects))
+ # Copy the file to the specific terraform directory
+ local("mkdir -p ./localdev/terraform/modules/vcs_files/base_files/appsets/{} && cp {} {}".format(appset, source_file, dest_file))
- k8s_resource(
- new_name=appset,
- objects=['{}:applicationset'.format(appset)],
- labels=["test_appsets"],
- resource_deps=["argocd-crds","argocd"],
- )
+ # Modify the copied file to replace ${REPO_URL} with projectUrl
+ local("sed -i '' 's#REPO_URL#{}#g' {}".format(projectUrl, dest_file))Feedback & Suggestions:
By addressing these points, the code will be more secure, robust, and maintainable. 🛡️🚀 😼 Mergecat review of pkg/affected_apps/multi_matcher_test.go@@ -6,14 +6,15 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/stretchr/testify/require"
+ "github.com/zapier/kubechecks/pkg/git"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
type fakeMatcher struct {
items AffectedItems
}
-func (f fakeMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
+func (f fakeMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string, repo *git.Repo) (AffectedItems, error) {
return f.items, nil
}
@@ -29,7 +30,7 @@ func TestMultiMatcher(t *testing.T) {
ctx := context.Background()
matcher := NewMultiMatcher(matcher1, matcher2)
- total, err := matcher.AffectedApps(ctx, nil, "")
+ total, err := matcher.AffectedApps(ctx, nil, "", nil)
require.NoError(t, err)
require.Len(t, total.Applications, 1)
@@ -47,7 +48,7 @@ func TestMultiMatcher(t *testing.T) {
ctx := context.Background()
matcher := NewMultiMatcher(matcher1, matcher2)
- total, err := matcher.AffectedApps(ctx, nil, "")
+ total, err := matcher.AffectedApps(ctx, nil, "", nil)
require.NoError(t, err)
require.Len(t, total.Applications, 1)
@@ -69,7 +70,7 @@ func TestMultiMatcher(t *testing.T) {
ctx := context.Background()
matcher := NewMultiMatcher(matcher1, matcher2)
- total, err := matcher.AffectedApps(ctx, nil, "")
+ total, err := matcher.AffectedApps(ctx, nil, "", nil)
require.NoError(t, err)
require.Len(t, total.Applications, 1)
@@ -92,7 +93,7 @@ func TestMultiMatcher(t *testing.T) {
ctx := context.Background()
matcher := NewMultiMatcher(matcher1, matcher2)
- total, err := matcher.AffectedApps(ctx, nil, "")
+ total, err := matcher.AffectedApps(ctx, nil, "", nil)
require.NoError(t, err)
require.Len(t, total.Applications, 2)Feedback & Suggestions:
😼 Mergecat review of docs/usage.md@@ -46,7 +46,9 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_ENABLE_PREUPGRADE`|Enable preupgrade checks.|`true`|
|`KUBECHECKS_ENSURE_WEBHOOKS`|Ensure that webhooks are created in repositories referenced by argo.|`false`|
|`KUBECHECKS_FALLBACK_K8S_VERSION`|Fallback target Kubernetes version for schema / upgrade checks.|`1.23.0`|
+|`KUBECHECKS_KUBERNETES_CLUSTERID`|Kubernetes Cluster ID, must be specified if kubernetes-type is eks.||
|`KUBECHECKS_KUBERNETES_CONFIG`|Path to your kubernetes config file, used to monitor applications.||
+|`KUBECHECKS_KUBERNETES_TYPE`|Kubernetes Type One of eks, or local.|`local`|
|`KUBECHECKS_LABEL_FILTER`|(Optional) If set, The label that must be set on an MR (as "kubechecks:<value>") for kubechecks to process the merge request webhook.||
|`KUBECHECKS_LOG_LEVEL`|Set the log output level. One of error, warn, info, debug, trace.|`info`|
|`KUBECHECKS_MAX_CONCURRENCT_CHECKS`|Number of concurrent checks to run.|`32`|
@@ -59,7 +61,7 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_PERSIST_LOG_LEVEL`|Persists the set log level down to other module loggers.|`false`|
|`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_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.|`[./schemas]`|
+|`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_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 cmd/root.go@@ -63,6 +63,11 @@ func init() {
stringFlag(flags, "argocd-api-namespace", "ArgoCD namespace where the application watcher will read Custom Resource Definitions (CRD) for Application and ApplicationSet resources.",
newStringOpts().
withDefault("argocd"))
+ stringFlag(flags, "kubernetes-type", "Kubernetes Type One of eks, or local. Defaults to local.",
+ newStringOpts().
+ withChoices("eks", "local").
+ withDefault("local"))
+ stringFlag(flags, "kubernetes-clusterid", "Kubernetes Cluster ID, must be specified if kubernetes-type is eks.")
stringFlag(flags, "kubernetes-config", "Path to your kubernetes config file, used to monitor applications.")
stringFlag(flags, "otel-collector-port", "The OpenTelemetry collector port.")
@@ -73,10 +78,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.",
- newStringSliceOpts().
- withDefault([]string{"./schemas"}))
-
+ 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.")
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().Feedback & Suggestions:
😼 Mergecat review of pkg/container/main.go@@ -5,6 +5,7 @@ import (
"io/fs"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ client "github.com/zapier/kubechecks/pkg/kubernetes"
"github.com/zapier/kubechecks/pkg"
"github.com/zapier/kubechecks/pkg/app_watcher"
@@ -16,23 +17,30 @@ import (
)
type Container struct {
- ApplicationWatcher *app_watcher.ApplicationWatcher
- ArgoClient *argo_client.ArgoClient
+ ApplicationWatcher *app_watcher.ApplicationWatcher
+ ApplicationSetWatcher *app_watcher.ApplicationSetWatcher
+ ArgoClient *argo_client.ArgoClient
Config config.ServerConfig
RepoManager *git.RepoManager
VcsClient vcs.Client
VcsToArgoMap 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
}Feedback & Suggestions:
Suggested changes: @@ -5,6 +5,7 @@ import (
"io/fs"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
- client "github.com/zapier/kubechecks/pkg/kubernetes"
"github.com/zapier/kubechecks/pkg"
"github.com/zapier/kubechecks/pkg/app_watcher"
@@ -16,23 +17,30 @@ import (
)
type Container struct {
- ApplicationWatcher *app_watcher.ApplicationWatcher
- ArgoClient *argo_client.ArgoClient
+ ApplicationWatcher *app_watcher.ApplicationWatcher
+ ApplicationSetWatcher *app_watcher.ApplicationSetWatcher
+ ArgoClient *argo_client.ArgoClient
Config config.ServerConfig
RepoManager *git.RepoManager
VcsClient vcs.Client
VcsToArgoMap 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
}😼 Mergecat review of pkg/appdir/vcstoargomap.go@@ -5,26 +5,31 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/rs/zerolog/log"
-
"github.com/zapier/kubechecks/pkg"
)
type VcsToArgoMap struct {
- username string
- appDirByRepo map[pkg.RepoURL]*AppDirectory
+ username string
+ appDirByRepo map[pkg.RepoURL]*AppDirectory
+ appSetDirByRepo map[pkg.RepoURL]*AppSetDirectory
}
func NewVcsToArgoMap(vcsUsername string) VcsToArgoMap {
return VcsToArgoMap{
- username: vcsUsername,
- appDirByRepo: make(map[pkg.RepoURL]*AppDirectory),
+ username: vcsUsername,
+ appDirByRepo: make(map[pkg.RepoURL]*AppDirectory),
+ appSetDirByRepo: make(map[pkg.RepoURL]*AppSetDirectory),
}
}
func (v2a VcsToArgoMap) GetMap() map[pkg.RepoURL]*AppDirectory {
return v2a.appDirByRepo
}
+func (v2a VcsToArgoMap) GetAppSetMap() map[pkg.RepoURL]*AppSetDirectory {
+ return v2a.appSetDirByRepo
+}
+
func (v2a VcsToArgoMap) GetAppsInRepo(repoCloneUrl string) *AppDirectory {
repoUrl, _, err := pkg.NormalizeRepoUrl(repoCloneUrl)
if err != nil {
@@ -40,6 +45,21 @@ func (v2a VcsToArgoMap) GetAppsInRepo(repoCloneUrl string) *AppDirectory {
return appdir
}
+// GetAppSetsInRepo returns AppSetDirectory for the specified repository URL.
+func (v2a VcsToArgoMap) GetAppSetsInRepo(repoCloneUrl string) *AppSetDirectory {
+ repoUrl, _, err := pkg.NormalizeRepoUrl(repoCloneUrl)
+ if err != nil {
+ log.Warn().Err(err).Msgf("failed to parse %s", repoCloneUrl)
+ }
+ appSetDir := v2a.appSetDirByRepo[repoUrl]
+ if appSetDir == nil {
+ appSetDir = NewAppSetDirectory()
+ v2a.appSetDirByRepo[repoUrl] = appSetDir
+ }
+
+ return appSetDir
+}
+
func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, fs fs.FS) *AppDirectory {
var (
err error
@@ -98,6 +118,41 @@ func (v2a VcsToArgoMap) GetVcsRepos() []string {
for key := range v2a.appDirByRepo {
repos = append(repos, key.CloneURL(v2a.username))
}
-
+ for key := range v2a.appSetDirByRepo {
+ repos = append(repos, key.CloneURL(v2a.username))
+ }
return repos
}
+
+func (v2a VcsToArgoMap) AddAppSet(app *v1alpha1.ApplicationSet) {
+ if app.Spec.Template.Spec.GetSource().RepoURL == "" {
+ log.Warn().Msgf("%s/%s: no source, skipping", app.Namespace, app.Name)
+ return
+ }
+
+ appDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL)
+ appDirectory.ProcessApp(*app)
+}
+
+func (v2a VcsToArgoMap) UpdateAppSet(old *v1alpha1.ApplicationSet, new *v1alpha1.ApplicationSet) {
+ if new.Spec.Template.Spec.GetSource().RepoURL == "" {
+ log.Warn().Msgf("%s/%s: no source, skipping", new.Namespace, new.Name)
+ return
+ }
+
+ oldAppDirectory := v2a.GetAppSetsInRepo(old.Spec.Template.Spec.GetSource().RepoURL)
+ oldAppDirectory.RemoveApp(*old)
+
+ newAppDirectory := v2a.GetAppSetsInRepo(new.Spec.Template.Spec.GetSource().RepoURL)
+ newAppDirectory.ProcessApp(*new)
+}
+
+func (v2a VcsToArgoMap) DeleteAppSet(app *v1alpha1.ApplicationSet) {
+ if app.Spec.Template.Spec.GetSource().RepoURL == "" {
+ log.Warn().Msgf("%s/%s: no source, skipping", app.Namespace, app.Name)
+ return
+ }
+
+ oldAppDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL)
+ oldAppDirectory.RemoveApp(*app)
+}Feedback & Suggestions:
😼 Mergecat review of pkg/affected_apps/argocd_matcher.go@@ -5,14 +5,14 @@ import (
"os"
"github.com/rs/zerolog/log"
-
"github.com/zapier/kubechecks/pkg/appdir"
"github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/git"
)
type ArgocdMatcher struct {
- appsDirectory *appdir.AppDirectory
+ appsDirectory *appdir.AppDirectory
+ appSetsDirectory *appdir.AppSetDirectory
}
func NewArgocdMatcher(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) (*ArgocdMatcher, error) {
@@ -23,8 +23,13 @@ func NewArgocdMatcher(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) (*Arg
Union(repoApps).
Union(kustomizeAppFiles)
+ repoAppSets := getArgocdAppSets(vcsToArgoMap, repo)
+ appSetDirectory := appdir.NewAppSetDirectory().
+ Union(repoAppSets)
+
return &ArgocdMatcher{
- appsDirectory: appDirectory,
+ appsDirectory: appDirectory,
+ appSetsDirectory: appSetDirectory,
}, nil
}
@@ -54,13 +59,31 @@ func getArgocdApps(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) *appdir.
return repoApps
}
-func (a *ArgocdMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
+func getArgocdAppSets(vcsToArgoMap container.VcsToArgoMap, repo *git.Repo) *appdir.AppSetDirectory {
+ log.Debug().Msgf("looking for %s repos", repo.CloneURL)
+ repoApps := vcsToArgoMap.GetAppSetsInRepo(repo.CloneURL)
+
+ if repoApps == nil {
+ log.Debug().Msg("found no appSets")
+ } else {
+ log.Debug().Msgf("found %d appSets", repoApps.Count())
+ }
+ return repoApps
+}
+
+func (a *ArgocdMatcher) AffectedApps(_ context.Context, changeList []string, targetBranch string, repo *git.Repo) (AffectedItems, error) {
if a.appsDirectory == nil {
return AffectedItems{}, nil
}
appsSlice := a.appsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch)
- return AffectedItems{Applications: appsSlice}, nil
+ appSetsSlice := a.appSetsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch, repo)
+
+ // and return both apps and appSets
+ return AffectedItems{
+ Applications: appsSlice,
+ ApplicationSets: appSetsSlice,
+ }, nil
}
var _ Matcher = new(ArgocdMatcher)Feedback & Suggestions:
By addressing these points, the code will be more robust, maintainable, and easier to debug. 🛠️✨ 😼 Mergecat review of pkg/events/check.go@@ -13,6 +13,8 @@ import (
"github.com/pkg/errors"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
+ "github.com/zapier/kubechecks/pkg/generator"
+ "github.com/zapier/kubechecks/pkg/repo_config"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
@@ -22,7 +24,6 @@ import (
"github.com/zapier/kubechecks/pkg/container"
"github.com/zapier/kubechecks/pkg/git"
"github.com/zapier/kubechecks/pkg/msg"
- "github.com/zapier/kubechecks/pkg/repo_config"
"github.com/zapier/kubechecks/pkg/vcs"
"github.com/zapier/kubechecks/telemetry"
)
@@ -49,13 +50,34 @@ type CheckEvent struct {
appsSent int32
appChannel chan *v1alpha1.Application
wg sync.WaitGroup
+ generator generator.AppsGenerator
+ matcher affected_apps.Matcher
}
type repoManager interface {
Clone(ctx context.Context, cloneURL, branchName string) (*git.Repo, error)
}
+func GenerateMatcher(ce *CheckEvent, repo *git.Repo) error {
+ log.Debug().Msg("using the argocd matcher")
+ m, err := affected_apps.NewArgocdMatcher(ce.ctr.VcsToArgoMap, repo)
+ if err != nil {
+ return errors.Wrap(err, "failed to create argocd matcher")
+ }
+ ce.matcher = m
+ cfg, err := repo_config.LoadRepoConfig(repo.Directory)
+ if err != nil {
+ return errors.Wrap(err, "failed to load repo config")
+ } else if cfg != nil {
+ log.Debug().Msg("using the config matcher")
+ configMatcher := affected_apps.NewConfigMatcher(cfg, ce.ctr)
+ ce.matcher = affected_apps.NewMultiMatcher(ce.matcher, configMatcher)
+ }
+ return nil
+}
+
func NewCheckEvent(pullRequest vcs.PullRequest, ctr container.Container, repoManager repoManager, processors []checks.ProcessorEntry) *CheckEvent {
+
ce := &CheckEvent{
addedAppsSet: make(map[string]v1alpha1.Application),
appChannel: make(chan *v1alpha1.Application, ctr.Config.MaxQueueSize),
@@ -64,6 +86,7 @@ func NewCheckEvent(pullRequest vcs.PullRequest, ctr container.Container, repoMan
processors: processors,
pullRequest: pullRequest,
repoManager: repoManager,
+ generator: generator.New(),
logger: log.Logger.With().
Str("repo", pullRequest.Name).
Int("event_id", pullRequest.CheckID).
@@ -87,34 +110,35 @@ func (ce *CheckEvent) UpdateListOfChangedFiles(ctx context.Context, repo *git.Re
return nil
}
+type MatcherFn func(ce *CheckEvent, repo *git.Repo) error
+
// GenerateListOfAffectedApps walks the repo to find any apps or appsets impacted by the changes in the MR/PR.
-func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context, repo *git.Repo, targetBranch string) error {
+func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context, repo *git.Repo, targetBranch string, initMatcherFn MatcherFn) error {
_, span := tracer.Start(ctx, "GenerateListOfAffectedApps")
defer span.End()
var err error
- var matcher affected_apps.Matcher
-
- log.Debug().Msg("using an argocd matcher")
- matcher, err = affected_apps.NewArgocdMatcher(ce.ctr.VcsToArgoMap, repo)
+ err = initMatcherFn(ce, repo)
if err != nil {
return errors.Wrap(err, "failed to create argocd matcher")
}
- cfg, err := repo_config.LoadRepoConfig(repo.Directory)
- if err != nil {
- return errors.Wrap(err, "failed to load repo config")
- } else if cfg != nil {
- log.Debug().Msg("using the config matcher")
- configMatcher := affected_apps.NewConfigMatcher(cfg, ce.ctr)
- matcher = affected_apps.NewMultiMatcher(matcher, configMatcher)
- }
-
- ce.affectedItems, err = matcher.AffectedApps(ctx, ce.fileList, targetBranch)
+ // use the changed file path to get the list of affected apps
+ // fileList is a list of changed files in the PR/MR, e.g. ["path/to/file1", "path/to/file2"]
+ ce.affectedItems, err = ce.matcher.AffectedApps(ctx, ce.fileList, targetBranch, repo)
if err != nil {
telemetry.SetError(span, err, "Get Affected Apps")
ce.logger.Error().Err(err).Msg("could not get list of affected apps and appsets")
}
+ for _, appSet := range ce.affectedItems.ApplicationSets {
+ apps, err := ce.generator.GenerateApplicationSetApps(ctx, appSet, &ce.ctr)
+ if err != nil {
+ ce.logger.Error().Err(err).Msg("could not generate apps from appSet")
+ continue
+ }
+ ce.affectedItems.Applications = append(ce.affectedItems.Applications, apps...)
+ }
+
span.SetAttributes(
attribute.Int("numAffectedApps", len(ce.affectedItems.Applications)),
attribute.Int("numAffectedAppSets", len(ce.affectedItems.ApplicationSets)),
@@ -149,7 +173,7 @@ func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneU
err error
repo *git.Repo
)
-
+ ce.logger.Info().Stack().Str("branchName", branchName).Msg("cloning repo")
ce.repoLock.Lock()
defer ce.repoLock.Unlock()
@@ -209,7 +233,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
_, span := tracer.Start(ctx, "GenerateListOfAffectedApps")
defer span.End()
- // Clone the repo's BaseRef (main etc) locally into the temp dir we just made
+ // 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)
if err != nil {
return errors.Wrap(err, "failed to clone repo")
@@ -226,7 +250,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
}
// Generate a list of affected apps, storing them within the CheckEvent (also returns but discarded here)
- if err = ce.GenerateListOfAffectedApps(ctx, repo, ce.pullRequest.BaseRef); err != nil {
+ if err = ce.GenerateListOfAffectedApps(ctx, repo, ce.pullRequest.BaseRef, GenerateMatcher); err != nil {
return errors.Wrap(err, "failed to generate a list of affected apps")
}
@@ -259,7 +283,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
}
go w.run(ctx)
}
-
+ ce.logger.Info().Msgf("adding %d apps to the queue", len(ce.affectedItems.Applications))
// Produce apps onto channel
for _, app := range ce.affectedItems.Applications {
ce.queueApp(app)
@@ -269,7 +293,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
close(ce.appChannel)
- ce.logger.Debug().Msg("finished an app")
+ ce.logger.Debug().Msg("finished an app/appsets")
ce.logger.Debug().
Int("all apps", len(ce.addedAppsSet)).Feedback & Suggestions:
Overall, the changes improve the readability and maintainability of the code. Good job! 🛠️ 😼 Mergecat review of go.mod@@ -7,6 +7,11 @@ toolchain go1.21.6
require (
github.com/argoproj/argo-cd/v2 v2.11.6
github.com/argoproj/gitops-engine v0.7.1-0.20240715141605-18ba62e1f1fb
+ github.com/aws/aws-sdk-go-v2 v1.30.1
+ github.com/aws/aws-sdk-go-v2/config v1.27.24
+ github.com/aws/aws-sdk-go-v2/service/eks v1.46.0
+ github.com/aws/aws-sdk-go-v2/service/sts v1.30.1
+ github.com/aws/smithy-go v1.20.3
github.com/cenkalti/backoff/v4 v4.3.0
github.com/chainguard-dev/git-urls v1.0.2
github.com/creasty/defaults v1.7.0
@@ -16,6 +21,8 @@ require (
github.com/go-logr/zerologr v1.2.3
github.com/google/go-github/v62 v62.0.0
github.com/heptiolabs/healthcheck v0.0.0-20211123025425-613501dd5deb
+ github.com/imdario/mergo v0.3.16
+ github.com/jeremywohl/flatten v1.0.1
github.com/labstack/echo-contrib v0.17.1
github.com/labstack/echo/v4 v4.12.0
github.com/masterminds/semver v1.5.0
@@ -50,8 +57,12 @@ require (
google.golang.org/grpc v1.64.0
gopkg.in/dealancer/validate.v2 v2.1.0
gopkg.in/yaml.v3 v3.0.1
+ k8s.io/api v0.26.15
+ k8s.io/apiextensions-apiserver v0.26.10
k8s.io/apimachinery v0.26.15
k8s.io/client-go v0.26.15
+ sigs.k8s.io/controller-runtime v0.14.7
+ sigs.k8s.io/yaml v1.4.0
)
require (
@@ -66,8 +77,10 @@ require (
github.com/CycloneDX/cyclonedx-go v0.8.0 // indirect
github.com/KeisukeYamashita/go-vcl v0.4.0 // indirect
github.com/MakeNowJust/heredoc v1.0.0 // indirect
+ github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver v1.5.0 // indirect
github.com/Masterminds/semver/v3 v3.2.1 // indirect
+ github.com/Masterminds/sprig/v3 v3.2.3 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
github.com/OneOfOne/xxhash v1.2.8 // indirect
github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371 // indirect
@@ -78,6 +91,15 @@ require (
github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
github.com/argoproj/pkg v0.13.7-0.20230627120311-a4dd357b057e // indirect
github.com/aws/aws-sdk-go v1.50.8 // indirect
+ github.com/aws/aws-sdk-go-v2/credentials v1.17.24 // indirect
+ github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.9 // indirect
+ github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.13 // indirect
+ github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.13 // indirect
+ github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
+ github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 // indirect
+ github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.15 // indirect
+ github.com/aws/aws-sdk-go-v2/service/sso v1.22.1 // indirect
+ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.2 // indirect
github.com/basgys/goxml2json v1.1.0 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
@@ -103,6 +125,7 @@ require (
github.com/emicklei/go-restful/v3 v3.10.2 // indirect
github.com/emirpasic/gods v1.18.1 // indirect
github.com/evanphx/json-patch v5.9.0+incompatible // indirect
+ github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect
github.com/fatih/camelcase v1.0.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
@@ -140,6 +163,8 @@ require (
github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
github.com/googleapis/gax-go/v2 v2.12.2 // indirect
github.com/gorilla/mux v1.8.1 // indirect
+ github.com/gosimple/slug v1.13.1 // indirect
+ github.com/gosimple/unidecode v1.0.1 // indirect
github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
@@ -151,7 +176,7 @@ require (
github.com/hashicorp/go-version v1.6.0 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/hashicorp/hcl/v2 v2.17.0 // indirect
- github.com/imdario/mergo v0.3.16 // indirect
+ github.com/huandu/xstrings v1.3.3 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/itchyny/gojq v0.12.13 // indirect
github.com/itchyny/timefmt-go v0.1.5 // indirect
@@ -175,9 +200,11 @@ require (
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-runewidth v0.0.15 // indirect
+ github.com/mitchellh/copystructure v1.0.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
+ github.com/mitchellh/reflectwalk v1.0.0 // indirect
github.com/moby/buildkit v0.12.5 // indirect
github.com/moby/docker-image-spec v1.3.1 // indirect
github.com/moby/spdystream v0.2.0 // indirect
@@ -208,6 +235,7 @@ require (
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 // indirect
github.com/sergi/go-diff v1.3.1 // indirect
+ github.com/shopspring/decimal v1.2.0 // indirect
github.com/shteou/go-ignore v0.3.1 // indirect
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 // indirect
github.com/skeema/knownhosts v1.2.2 // indirect
@@ -260,8 +288,6 @@ require (
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
- k8s.io/api v0.26.15 // indirect
- k8s.io/apiextensions-apiserver v0.26.10 // indirect
k8s.io/apiserver v0.26.15 // indirect
k8s.io/cli-runtime v0.26.15 // indirect
k8s.io/component-base v0.26.15 // indirect
@@ -276,12 +302,10 @@ require (
muzzammil.xyz/jsonc v1.0.0 // indirect
olympos.io/encoding/edn v0.0.0-20201019073823-d3554ca0b0a3 // indirect
oras.land/oras-go/v2 v2.3.1 // indirect
- sigs.k8s.io/controller-runtime v0.14.7 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/kustomize/api v0.12.1 // indirect
sigs.k8s.io/kustomize/kyaml v0.13.9 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
- sigs.k8s.io/yaml v1.4.0 // indirect
)
replace (Feedback & Suggestions:
To improve the 😼 Mergecat review of cmd/container.go@@ -12,6 +12,7 @@ import (
"github.com/zapier/kubechecks/pkg/config"
"github://github.com/zapier/kubechecks/pkg/container"
"github://github.com/zapier/kubechecks/pkg/git"
+ client "github://github.com/zapier/kubechecks/pkg/kubernetes"
"github://github.com/zapier/kubechecks/pkg/vcs/github_client"
"github://github.com/zapier/kubechecks/pkg/vcs/gitlab_client"
)
@@ -36,7 +37,30 @@ func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool)
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); err != nil {
return ctr, errors.Wrap(err, "failed to create argo client")
@@ -52,13 +76,22 @@ func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool)
return ctr, errors.Wrap(err, "failed to build apps map")
}
+ if err = buildAppSetsMap(ctx, ctr.ArgoClient, ctr.VcsToArgoMap); err != nil {
+ return ctr, errors.Wrap(err, "failed to build appsets map")
+ }
+
if watchApps {
- ctr.ApplicationWatcher, err = app_watcher.NewApplicationWatcher(vcsToArgoMap, cfg)
+ ctr.ApplicationWatcher, err = app_watcher.NewApplicationWatcher(kubeClient.Config(), vcsToArgoMap, cfg)
if err != nil {
return ctr, errors.Wrap(err, "failed to create watch applications")
}
+ ctr.ApplicationSetWatcher, err = app_watcher.NewApplicationSetWatcher(kubeClient.Config(), vcsToArgoMap, cfg)
+ if err != nil {
+ return ctr, errors.Wrap(err, "failed to create watch application sets")
+ }
go ctr.ApplicationWatcher.Run(ctx, 1)
+ go ctr.ApplicationSetWatcher.Run(ctx)
}
} else {
log.Info().Msgf("not monitoring applications, MonitorAllApplications: %+v", cfg.MonitorAllApplications)
@@ -75,6 +108,16 @@ func buildAppsMap(ctx context.Context, argoClient *argo_client.ArgoClient, resul
for _, app := range apps.Items {
result.AddApp(&app)
}
+ return nil
+}
+func buildAppSetsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result container.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:
Here is a revised version of the diff with these suggestions: @@ -12,6 +12,7 @@ import (
"github.com/zapier/kubechecks/pkg/config"
"github://github.com/zapier/kubechecks/pkg/container"
"github://github.com/zapier/kubechecks/pkg/git"
- client "github://github.com/zapier/kubechecks/pkg/kubernetes"
"github://github.com/zapier/kubechecks/pkg/vcs/github_client"
"github://github.com/zapier/kubechecks/pkg/vcs/gitlab_client"
)
@@ -36,7 +37,30 @@ func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool)
if err != nil {
return ctr, errors.Wrap(err, "failed to create vcs client")
}
+ var kubeClient client.Interface
+ kubeClient, err = createKubeClient(ctx, cfg)
+ 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); err != nil {
return ctr, errors.Wrap(err, "failed to create argo client")
@@ -52,13 +76,22 @@ func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool)
return ctr, errors.Wrap(err, "failed to build apps map")
}
+ if err = buildAppSetsMap(ctx, ctr.ArgoClient, ctr.VcsToArgoMap); err != nil {
+ return ctr, errors.Wrap(err, "failed to build appsets map")
+ }
+
if watchApps {
- ctr.ApplicationWatcher, err = app_watcher.NewApplicationWatcher(vcsToArgoMap, cfg)
+ ctr.ApplicationWatcher, err = app_watcher.NewApplicationWatcher(kubeClient.Config(), vcsToArgoMap, cfg)
if err != nil {
return ctr, errors.Wrap(err, "failed to create watch applications")
}
+ ctr.ApplicationSetWatcher, err = app_watcher.NewApplicationSetWatcher(kubeClient.Config(), vcsToArgoMap, cfg)
+ if err != nil {
+ return ctr, errors.Wrap(err, "failed to create watch application sets")
+ }
go ctr.ApplicationWatcher.Run(ctx, 1)
+ go ctr.ApplicationSetWatcher.Run(ctx)
}
} else {
log.Info().Msgf("not monitoring applications, MonitorAllApplications: %+v", cfg.MonitorAllApplications)
@@ -75,6 +108,16 @@ func buildAppsMap(ctx context.Context, argoClient *argo_client.ArgoClient, resul
for _, app := range apps.Items {
result.AddApp(&app)
}
+ return nil
+}
+func buildAppSetsMap(ctx context.Context, argoClient *argo_client.ArgoClient, result container.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
}
+
+func createKubeClient(ctx context.Context, cfg config.ServerConfig) (client.Interface, error) {
+ var kubeClient client.Interface
+ var err error
+ switch cfg.KubernetesType {
+ // TODO: expand with other cluster types
+ case client.ClusterTypeLOCAL:
+ kubeClient, err = client.New(&client.NewClientInput{
+ KubernetesConfigPath: cfg.KubernetesConfig,
+ ClusterType: cfg.KubernetesType,
+ })
+ case client.ClusterTypeEKS:
+ kubeClient, err = client.New(&client.NewClientInput{
+ KubernetesConfigPath: cfg.KubernetesConfig,
+ ClusterType: cfg.KubernetesType,
+ },
+ client.EKSClientOption(ctx, cfg.KubernetesClusterID),
+ )
+ }
+ return kubeClient, err
+}This revised diff addresses the redundancy, improves error handling, and adds a new function 😼 Mergecat review of pkg/events/check_test.go@@ -6,11 +6,22 @@ import (
"fmt"
"testing"
+ "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+ "github.com/rs/zerolog"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
-
+ affectedappsmocks "github.com/zapier/kubechecks/mocks/affected_apps/mocks"
+ generatorsmocks "github.com/zapier/kubechecks/mocks/generator/mocks"
+ "github.com/zapier/kubechecks/pkg/affected_apps"
+ "github.com/zapier/kubechecks/pkg/checks"
"github.com/zapier/kubechecks/pkg/config"
+ "github.com/zapier/kubechecks/pkg/container"
+ "github.com/zapier/kubechecks/pkg/generator"
"github.com/zapier/kubechecks/pkg/git"
+ "github.com/zapier/kubechecks/pkg/msg"
+ "github.com/zapier/kubechecks/pkg/vcs"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
// TestCleanupGetManifestsError tests the cleanupGetManifestsError function.
@@ -123,3 +134,182 @@ func TestCheckEventGetRepo(t *testing.T) {
assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "gh-pages"))
})
}
+
+func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) {
+ type fields struct {
+ fileList []string
+ pullRequest vcs.PullRequest
+ logger zerolog.Logger
+ vcsNote *msg.Message
+ affectedItems affected_apps.AffectedItems
+ ctr container.Container
+ repoManager repoManager
+ processors []checks.ProcessorEntry
+ clonedRepos map[string]*git.Repo
+ addedAppsSet map[string]v1alpha1.Application
+ appsSent int32
+ appChannel chan *v1alpha1.Application
+ generator generator.AppsGenerator
+ matcher affected_apps.Matcher
+ }
+ type args struct {
+ ctx context.Context
+ repo *git.Repo
+ targetBranch string
+ initMatcherFn MatcherFn
+ }
+ tests := []struct {
+ name string
+ fields fields
+ args args
+ expectedAppCount int
+ wantErr assert.ErrorAssertionFunc
+ }{
+ // TODO: Add test cases.
+ {
+ name: "no error",
+ fields: fields{
+ fileList: nil,
+ pullRequest: vcs.PullRequest{},
+ logger: zerolog.Logger{},
+ vcsNote: nil,
+ affectedItems: affected_apps.AffectedItems{},
+ ctr: container.Container{},
+ repoManager: nil,
+ processors: nil,
+ clonedRepos: nil,
+ addedAppsSet: nil,
+ appsSent: 0,
+ appChannel: nil,
+ generator: MockGenerator("GenerateApplicationSetApps", []interface{}{[]v1alpha1.Application{}, nil}),
+ matcher: MockMatcher("AffectedApps", []interface{}{
+ affected_apps.AffectedItems{
+ ApplicationSets: []v1alpha1.ApplicationSet{
+ {
+ TypeMeta: metav1.TypeMeta{Kind: "ApplicationSet", APIVersion: "argoproj.io/v1alpha1"},
+ ObjectMeta: metav1.ObjectMeta{Name: "appset1"},
+ },
+ },
+ },
+ nil,
+ }),
+ },
+ args: args{
+ ctx: context.Background(),
+ repo: &git.Repo{Directory: "/tmp"},
+ targetBranch: "HEAD",
+ initMatcherFn: MockInitMatcherFn(),
+ },
+ expectedAppCount: 1,
+ wantErr: assert.NoError,
+ },
+ {
+ name: "matcher error",
+ fields: fields{
+ fileList: nil,
+ pullRequest: vcs.PullRequest{},
+ logger: zerolog.Logger{},
+ vcsNote: nil,
+ affectedItems: affected_apps.AffectedItems{},
+ ctr: container.Container{},
+ repoManager: nil,
+ processors: nil,
+ clonedRepos: nil,
+ addedAppsSet: nil,
+ appsSent: 0,
+ appChannel: nil,
+ generator: MockGenerator("GenerateApplicationSetApps", []interface{}{[]v1alpha1.Application{}, nil}),
+ matcher: MockMatcher("AffectedApps", []interface{}{
+ affected_apps.AffectedItems{},
+ fmt.Errorf("mock error"),
+ }),
+ },
+ args: args{
+ ctx: context.Background(),
+ repo: &git.Repo{Directory: "/tmp"},
+ targetBranch: "HEAD",
+ initMatcherFn: MockInitMatcherFn(),
+ },
+ expectedAppCount: 0,
+ wantErr: assert.Error,
+ },
+ {
+ name: "generator error",
+ fields: fields{
+ fileList: nil,
+ pullRequest: vcs.PullRequest{},
+ logger: zerolog.Logger{},
+ vcsNote: nil,
+ affectedItems: affected_apps.AffectedItems{},
+ ctr: container.Container{},
+ repoManager: nil,
+ processors: nil,
+ clonedRepos: nil,
+ addedAppsSet: nil,
+ appsSent: 0,
+ appChannel: nil,
+ generator: MockGenerator("GenerateApplicationSetApps", []interface{}{[]v1alpha1.Application{}, fmt.Errorf("mock error")}),
+ matcher: MockMatcher("AffectedApps", []interface{}{
+ affected_apps.AffectedItems{
+ ApplicationSets: []v1alpha1.ApplicationSet{
+ {
+ TypeMeta: metav1.TypeMeta{Kind: "ApplicationSet", APIVersion: "argoproj.io/v1alpha1"},
+ ObjectMeta: metav1.ObjectMeta{Name: "appset1"},
+ },
+ },
+ },
+ nil,
+ }),
+ },
+ args: args{
+ ctx: context.Background(),
+ repo: &git.Repo{Directory: "/tmp"},
+ targetBranch: "HEAD",
+ initMatcherFn: MockInitMatcherFn(),
+ },
+ expectedAppCount: 0,
+ wantErr: assert.NoError,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ ce := &CheckEvent{
+ fileList: tt.fields.fileList,
+ pullRequest: tt.fields.pullRequest,
+ logger: tt.fields.logger,
+ vcsNote: tt.fields.vcsNote,
+ affectedItems: tt.fields.affectedItems,
+ ctr: tt.fields.ctr,
+ repoManager: tt.fields.repoManager,
+ processors: tt.fields.processors,
+ clonedRepos: tt.fields.clonedRepos,
+ addedAppsSet: tt.fields.addedAppsSet,
+ appsSent: tt.fields.appsSent,
+ appChannel: tt.fields.appChannel,
+ generator: tt.fields.generator,
+ matcher: tt.fields.matcher,
+ }
+ tt.wantErr(t, ce.GenerateListOfAffectedApps(tt.args.ctx, tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn), fmt.Sprintf("GenerateListOfAffectedApps(%v, %v, %v, %v)", tt.args.ctx, tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn))
+
+ })
+ }
+}
+
+func MockMatcher(methodName string, returns []interface{}) affected_apps.Matcher {
+ mockClient := new(affectedappsmocks.MockMatcher)
+ mockClient.On(methodName, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(returns...)
+
+ return mockClient
+}
+
+func MockGenerator(methodName string, returns []interface{}) generator.AppsGenerator {
+ mockClient := new(generatorsmocks.MockAppsGenerator)
+ mockClient.On(methodName, mock.Anything, mock.Anything, mock.Anything).Return(returns...)
+
+ return mockClient
+}
+func MockInitMatcherFn() MatcherFn {
+ return func(ce *CheckEvent, repo *git.Repo) error {
+ return nil
+ }
+}Feedback & Suggestions:
By addressing these points, the code will be cleaner, more maintainable, and less prone to errors. 🛠️ Dependency ReviewClick to read mergecats review!No suggestions found |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change
Local Dev Change