From 223311dabb69d7b5cb484d85b4471bfb15acfb78 Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Tue, 8 Oct 2024 16:41:37 -0400 Subject: [PATCH 1/6] add nil condition to found app to prevent nil references --- internal/pkg/argocd/argocd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index c4ef8039..0f94f52c 100644 --- a/internal/pkg/argocd/argocd.go +++ b/internal/pkg/argocd/argocd.go @@ -324,7 +324,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st return fmt.Errorf("Error creating ArgoCD clients: %w", err) } foundApp, err = findArgocdApp(ctx, componentPath, repo, ac.app, useSHALabelForArgoDicovery) - if err != nil { + if foundApp == nil || err != nil { return fmt.Errorf("error finding ArgoCD application for component path %s: %w", componentPath, err) } if foundApp.Spec.Source.TargetRevision == revision { From 09094d4c02602d89a72b42d394c652d4ada19fa0 Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Tue, 8 Oct 2024 17:03:55 -0400 Subject: [PATCH 2/6] add templates path to generate argo cd diff comments test --- internal/pkg/githubapi/github_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go index a2d965aa..e35007e5 100644 --- a/internal/pkg/githubapi/github_test.go +++ b/internal/pkg/githubapi/github_test.go @@ -186,6 +186,10 @@ func TestGenerateArgoCdDiffComments(t *testing.T) { }, } + if err := os.Setenv("TEMPLATES_PATH", "../../../templates/"); err != nil { + t.Fatal(err) + } + for name, tc := range tests { tc := tc // capture range variable name := name From 1a5819d6d47f7e0453da5c41b5d66e08702041e2 Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Wed, 9 Oct 2024 08:26:06 -0400 Subject: [PATCH 3/6] add nolint --- internal/pkg/githubapi/github_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/githubapi/github_test.go b/internal/pkg/githubapi/github_test.go index e35007e5..1527c75a 100644 --- a/internal/pkg/githubapi/github_test.go +++ b/internal/pkg/githubapi/github_test.go @@ -186,7 +186,7 @@ func TestGenerateArgoCdDiffComments(t *testing.T) { }, } - if err := os.Setenv("TEMPLATES_PATH", "../../../templates/"); err != nil { + if err := os.Setenv("TEMPLATES_PATH", "../../../templates/"); err != nil { //nolint:tenv t.Fatal(err) } From 9874680d86cc9655a6265db985e7a7c7d228cde3 Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Wed, 9 Oct 2024 08:43:53 -0400 Subject: [PATCH 4/6] separate error and nil conditions for clarity --- internal/pkg/argocd/argocd.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index 0f94f52c..e1dbf96a 100644 --- a/internal/pkg/argocd/argocd.go +++ b/internal/pkg/argocd/argocd.go @@ -324,9 +324,12 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st return fmt.Errorf("Error creating ArgoCD clients: %w", err) } foundApp, err = findArgocdApp(ctx, componentPath, repo, ac.app, useSHALabelForArgoDicovery) - if foundApp == nil || err != nil { + if err != nil { return fmt.Errorf("error finding ArgoCD application for component path %s: %w", componentPath, err) } + if foundApp == nil { + return fmt.Errorf("no ArgoCD application was found for component path: %s", componentPath) + } if foundApp.Spec.Source.TargetRevision == revision { log.Infof("App %s already has revision %s", foundApp.Name, revision) return nil From 7188f6bbcc66dcb9c28927103aaa6927de3f1a26 Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Tue, 15 Oct 2024 16:09:02 -0400 Subject: [PATCH 5/6] add a unit test to verify the nil return value --- internal/pkg/argocd/argocd_test.go | 38 +++++++++++++++++++++++++++++ internal/pkg/testutils/testutils.go | 16 ++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 internal/pkg/testutils/testutils.go diff --git a/internal/pkg/argocd/argocd_test.go b/internal/pkg/argocd/argocd_test.go index dc78cecc..3d461a2e 100644 --- a/internal/pkg/argocd/argocd_test.go +++ b/internal/pkg/argocd/argocd_test.go @@ -3,6 +3,7 @@ package argocd import ( "bytes" "context" + "log" "os" "strings" "testing" @@ -11,6 +12,7 @@ import ( argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/golang/mock/gomock" "github.com/wayfair-incubator/telefonistka/internal/pkg/mocks" + "github.com/wayfair-incubator/telefonistka/internal/pkg/testutils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -273,3 +275,39 @@ func TestFindArgocdAppByPathAnnotationRelative2(t *testing.T) { t.Errorf("App name is not right-app") } } + +func TestFindArgocdAppByPathAnnotationNotFound(t *testing.T) { + t.Parallel() + defer testutils.Quiet()() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "wrong-path", + }, + Name: "right-app", + }, + Spec: argoappv1.ApplicationSpec{ + Source: &argoappv1.ApplicationSource{ + RepoURL: "", + Path: "right/", + }, + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } + if app != nil { + log.Fatal("expected the application to be nil") + } +} diff --git a/internal/pkg/testutils/testutils.go b/internal/pkg/testutils/testutils.go new file mode 100644 index 00000000..b7673a67 --- /dev/null +++ b/internal/pkg/testutils/testutils.go @@ -0,0 +1,16 @@ +package testutils + +import ( + "io" + "os" + + log "github.com/sirupsen/logrus" +) + +// Quiet suppresses logs when running go test. +func Quiet() func() { + log.SetOutput(io.Discard) + return func() { + log.SetOutput(os.Stdout) + } +} From dbc0da89ab7816e45d857e056c5733934849c72a Mon Sep 17 00:00:00 2001 From: Jared Patrick Date: Wed, 16 Oct 2024 15:33:34 -0400 Subject: [PATCH 6/6] update test for clarity --- internal/pkg/argocd/argocd_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/argocd/argocd_test.go b/internal/pkg/argocd/argocd_test.go index 3d461a2e..80e094d9 100644 --- a/internal/pkg/argocd/argocd_test.go +++ b/internal/pkg/argocd/argocd_test.go @@ -288,14 +288,14 @@ func TestFindArgocdAppByPathAnnotationNotFound(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "argocd.argoproj.io/manifest-generate-paths": "wrong-path", + "argocd.argoproj.io/manifest-generate-paths": "non-existing-path", }, - Name: "right-app", + Name: "non-existing-app", }, Spec: argoappv1.ApplicationSpec{ Source: &argoappv1.ApplicationSource{ RepoURL: "", - Path: "right/", + Path: "non-existing/", }, }, }, @@ -303,7 +303,7 @@ func TestFindArgocdAppByPathAnnotationNotFound(t *testing.T) { } mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) - app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + app, err := findArgocdAppByManifestPathAnnotation(ctx, "non-existing/path", "some-repo", mockApplicationClient) if err != nil { t.Errorf("Error: %v", err) }