Skip to content

Commit

Permalink
Add test for deleting secret with PipelineRun
Browse files Browse the repository at this point in the history
and some refactoring along the way. I separated the commits to make it
easier to review from the implementation.
  • Loading branch information
chmouel committed Jun 30, 2023
1 parent 1c02d06 commit 1c67e77
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 54 deletions.
12 changes: 6 additions & 6 deletions pkg/cmd/tknpac/describe/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestDescribe(t *testing.T) {
currentNamespace: ns,
opts: &describeOpts{},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "running", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "tartanpion",
keys.EventType: "papayolo",
Expand Down Expand Up @@ -89,7 +89,7 @@ func TestDescribe(t *testing.T) {
currentNamespace: ns,
opts: &describeOpts{},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "running", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "tartanpion",
}, 30),
Expand All @@ -105,11 +105,11 @@ func TestDescribe(t *testing.T) {
currentNamespace: ns,
opts: &describeOpts{TargetPipelineRun: "running2"},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "running", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "tartanpion",
}, 30),
tektontest.MakePRCompletion(cw, "running2", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running2", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "vavaroom",
}, 30),
Expand All @@ -125,11 +125,11 @@ func TestDescribe(t *testing.T) {
currentNamespace: ns,
opts: &describeOpts{},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "running", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "tartanpion",
}, 30),
tektontest.MakePRCompletion(cw, "running2", ns, running, map[string]string{
tektontest.MakePRCompletion(cw, "running2", ns, running, nil, map[string]string{
keys.Repository: "test-run",
keys.Branch: "vavaroom",
}, 30),
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/tknpac/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestList(t *testing.T) {
},
repositories: []*pacv1alpha1.Repository{repoNamespace1},
pipelineruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "running", namespace1.GetName(), running, map[string]string{
tektontest.MakePRCompletion(cw, "running", namespace1.GetName(), running, nil, map[string]string{
keys.Repository: repoNamespace1.GetName(),
keys.SHA: repoNamespace1SHA,
}, 30),
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/tknpac/logs/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestLogs(t *testing.T) {
currentNamespace: ns,
shift: 0,
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, map[string]string{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, nil, map[string]string{
keys.Repository: "test",
}, 30),
},
Expand All @@ -56,10 +56,10 @@ func TestLogs(t *testing.T) {
currentNamespace: ns,
useLastPR: true,
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, map[string]string{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, nil, map[string]string{
keys.Repository: "test",
}, 30),
tektontest.MakePRCompletion(cw, "test-pipeline2", ns, completed, map[string]string{
tektontest.MakePRCompletion(cw, "test-pipeline2", ns, completed, nil, map[string]string{
keys.Repository: "test",
}, 30),
},
Expand All @@ -71,7 +71,7 @@ func TestLogs(t *testing.T) {
currentNamespace: ns,
shift: 2,
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, map[string]string{
tektontest.MakePRCompletion(cw, "test-pipeline", ns, completed, nil, map[string]string{
keys.Repository: "test",
}, 30),
},
Expand Down
5 changes: 4 additions & 1 deletion pkg/kubeinteraction/cleanups.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ func (k Interaction) CleanupPipelines(ctx context.Context, logger *zap.SugaredLo
// Try to Delete the secret created for git-clone basic-auth, it should have been created with a owneref on the pipelinerun and due being deleted when the pipelinerun is deleted
// but in some cases of conflicts and the ownerRef not being set, the secret is not deleted and we need to delete it manually.
if secretName, ok := pr.GetAnnotations()[keys.GitAuthSecret]; ok {
_ = k.Run.Clients.Kube.CoreV1().Secrets(repo.GetNamespace()).Delete(ctx, secretName, metav1.DeleteOptions{})
err = k.Run.Clients.Kube.CoreV1().Secrets(repo.GetNamespace()).Delete(ctx, secretName, metav1.DeleteOptions{})
if err == nil {
logger.Infof("secret %s attached to pipelinerun %s has been deleted", secretName, prun.GetName())
}
}
}
}
Expand Down
70 changes: 57 additions & 13 deletions pkg/kubeinteraction/cleanups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"go.uber.org/zap"
zapobserver "go.uber.org/zap/zaptest/observer"
"golang.org/x/exp/maps"
"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -27,21 +28,21 @@ func TestCleanupPipelines(t *testing.T) {
keys.OriginalPRName: cleanupPRName,
keys.Repository: cleanupRepoName,
}
cleanupAnnotation := map[string]string{
keys.OriginalPRName: cleanupPRName,
keys.Repository: cleanupRepoName,
}
// copy of cleanupLabels to be used in annotations
cleanupAnnotations := maps.Clone(cleanupLabels)

clock := clockwork.NewFakeClock()

type args struct {
logSnippet string
namespace string
repositoryName string
maxKeep int
pruns []*tektonv1.PipelineRun
prunCurrent *tektonv1.PipelineRun
kept int
prunLatestInList string
secrets []*corev1.Secret
}

tests := []struct {
Expand All @@ -56,11 +57,11 @@ func TestCleanupPipelines(t *testing.T) {
repositoryName: cleanupRepoName,
maxKeep: 1,
kept: 1,
prunCurrent: &tektonv1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Labels: cleanupLabels, Annotations: cleanupAnnotation}},
prunCurrent: &tektonv1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Labels: cleanupLabels, Annotations: cleanupAnnotations}},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
},
prunLatestInList: "pipeline-newest",
},
Expand All @@ -72,15 +73,54 @@ func TestCleanupPipelines(t *testing.T) {
repositoryName: cleanupRepoName,
maxKeep: 1,
kept: 1, // see my comment in code why only 1 is kept.
prunCurrent: &tektonv1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Labels: cleanupLabels, Annotations: cleanupAnnotation}},
prunCurrent: &tektonv1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Labels: cleanupLabels, Annotations: cleanupAnnotations}},
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-running", ns, tektonv1.PipelineRunReasonRunning.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-toclean", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-tokeep", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-running", ns, tektonv1.PipelineRunReasonRunning.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-toclean", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-tokeep", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
},
prunLatestInList: "pipeline-running",
},
},
{
name: "cleanup with secrets",
args: args{
namespace: ns,
repositoryName: cleanupRepoName,
logSnippet: "secret pac-gitauth-secret attached to pipelinerun pipeline-toclean has been deleted",
prunCurrent: &tektonv1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Labels: cleanupLabels,
Annotations: map[string]string{
keys.OriginalPRName: cleanupPRName,
keys.Repository: cleanupRepoName,
keys.GitAuthSecret: "pac-gitauth-secret",
},
},
},
maxKeep: 0,
kept: 0,
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-toclean", ns, tektonv1.PipelineRunReasonSuccessful.String(), map[string]string{
keys.OriginalPRName: cleanupPRName,
keys.Repository: cleanupRepoName,
keys.GitAuthSecret: "pac-gitauth-secret",
}, cleanupLabels, 30),
},
secrets: []*corev1.Secret{
{
ObjectMeta: metav1.ObjectMeta{
Name: "pac-gitauth-secret",
Namespace: ns,
},
Data: map[string][]byte{
"username": []byte("test"),
"password": []byte("test"),
},
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -101,9 +141,10 @@ func TestCleanupPipelines(t *testing.T) {
},
},
},
Secret: tt.args.secrets,
}
stdata, _ := testclient.SeedTestData(t, ctx, tdata)
observer, _ := zapobserver.New(zap.InfoLevel)
observer, logCatcher := zapobserver.New(zap.InfoLevel)
fakelogger := zap.New(observer).Sugar()
kint := Interaction{
Run: &params.Run{
Expand All @@ -126,6 +167,9 @@ func TestCleanupPipelines(t *testing.T) {
if tt.args.prunLatestInList != "" {
assert.Equal(t, tt.args.prunLatestInList, plist.Items[0].Name)
}
if tt.args.logSnippet != "" {
assert.Assert(t, logCatcher.FilterMessageSnippet(tt.args.logSnippet).Len() > 0, logCatcher.All())
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/kubeinteraction/status/task_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
exitcode = 1
}

pr := tektontest.MakePRCompletion(clock, "pipeline-newest", "ns", tektonv1.PipelineRunReasonSuccessful.String(), make(map[string]string), 10)
pr := tektontest.MakePRCompletion(clock, "pipeline-newest", "ns", tektonv1.PipelineRunReasonSuccessful.String(), nil, make(map[string]string), 10)
pr.Status.ChildReferences = []tektonv1.ChildStatusReference{
{
TypeMeta: runtime.TypeMeta{
Expand Down
24 changes: 12 additions & 12 deletions pkg/reconciler/cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ func TestCleanupPipelineRuns(t *testing.T) {
{
name: "using from annotation",
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
},
repoNs: ns,
repoName: cleanupRepoName,
Expand All @@ -75,9 +75,9 @@ func TestCleanupPipelineRuns(t *testing.T) {
{
name: "using from config, as annotation value is more than config",
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
},
repoNs: ns,
repoName: cleanupRepoName,
Expand All @@ -92,9 +92,9 @@ func TestCleanupPipelineRuns(t *testing.T) {
{
name: "using from annotation, as annotation value is less than config",
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
},
repoNs: ns,
repoName: cleanupRepoName,
Expand All @@ -109,9 +109,9 @@ func TestCleanupPipelineRuns(t *testing.T) {
{
name: "no max-keep-runs annotation, using default from config",
pruns: []*tektonv1.PipelineRun{
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), cleanupLabels, 30),
tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 10),
tektontest.MakePRCompletion(clock, "pipeline-middest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 20),
tektontest.MakePRCompletion(clock, "pipeline-oldest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, cleanupLabels, 30),
},
repoNs: ns,
repoName: cleanupRepoName,
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestReconciler_ReconcileKind(t *testing.T) {
if tt.finalStatus == finalFailureStatus {
statusPR = tektonv1.PipelineRunReasonFailed
}
pr := tektontest.MakePRCompletion(clock, "pipeline-newest", "ns", string(statusPR), make(map[string]string), 10)
pr := tektontest.MakePRCompletion(clock, "pipeline-newest", "ns", string(statusPR), nil, make(map[string]string), 10)
pr.Status.ChildReferences = []tektonv1.ChildStatusReference{
{
TypeMeta: runtime.TypeMeta{
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestPostFinalStatus(t *testing.T) {
labels := map[string]string{}
ns := "namespace"
clock := clockwork.NewFakeClock()
pr1 := tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), labels, 10)
pr1 := tektontest.MakePRCompletion(clock, "pipeline-newest", ns, tektonv1.PipelineRunReasonSuccessful.String(), nil, labels, 10)
pr1.Status.Conditions = append(pr1.Status.Conditions, apis.Condition{Status: "False", Message: "Hello not good", Type: "Succeeded", Reason: "CouldntGetTask"})
ctx, _ := rtesting.SetupFakeContext(t)
tdata := testclient.Data{PipelineRuns: []*tektonv1.PipelineRun{pr1}}
Expand Down
24 changes: 12 additions & 12 deletions pkg/sort/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ func TestPipelineRunSortByCompletionTime(t *testing.T) {
}{
{
pruns: []tektonv1.PipelineRun{
*(tektontest.MakePRCompletion(clock, "troisieme", ns, success, labels, 30)),
*(tektontest.MakePRCompletion(clock, "premier", ns, success, labels, 10)),
*(tektontest.MakePRCompletion(clock, "second", ns, success, labels, 20)),
*(tektontest.MakePRCompletion(clock, "troisieme", ns, success, nil, labels, 30)),
*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
*(tektontest.MakePRCompletion(clock, "second", ns, success, nil, labels, 20)),
},
wantName: []string{"premier", "second", "troisieme"},
},
Expand All @@ -45,14 +45,14 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
ns := "namespace"
labels := map[string]string{}
success := tektonv1.PipelineRunReasonSuccessful.String()
startedEarlierPR := tektontest.MakePRCompletion(clock, "earlier", ns, success, labels, 5)
startedEarlierPR := tektontest.MakePRCompletion(clock, "earlier", ns, success, nil, labels, 5)
startedEarlierPR.Status.StartTime = &metav1.Time{Time: clock.Now().Add(100 * time.Minute)}

noCompletionPR := tektontest.MakePRCompletion(clock, "noCompletion", ns, success, labels, 5)
noCompletionPR := tektontest.MakePRCompletion(clock, "noCompletion", ns, success, nil, labels, 5)
noCompletionPR.Status.StartTime = &metav1.Time{Time: clock.Now().Add(500 * time.Minute)}
noCompletionPR.Status.CompletionTime = nil

notStartedYet := tektontest.MakePRCompletion(clock, "notStarted", ns, success, labels, 5)
notStartedYet := tektontest.MakePRCompletion(clock, "notStarted", ns, success, nil, labels, 5)
noCompletionPR.Status.StartTime = nil
noCompletionPR.Status.CompletionTime = nil

Expand All @@ -64,8 +64,8 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
{
name: "finished last started first",
pruns: []tektonv1.PipelineRun{
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, labels, 10)),
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, nil, labels, 10)),
*startedEarlierPR,
},
wantName: []string{"earlier", "otherFirst", "otherSecond"},
Expand All @@ -74,8 +74,8 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
name: "no completion but started first",
pruns: []tektonv1.PipelineRun{
*noCompletionPR,
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, labels, 10)),
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, nil, labels, 10)),
},
wantName: []string{"noCompletion", "otherFirst", "otherSecond"},
},
Expand All @@ -84,8 +84,8 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
name: "not started yet",
pruns: []tektonv1.PipelineRun{
*notStartedYet,
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, labels, 10)),
*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, nil, labels, 10)),
},
wantName: []string{"otherFirst", "otherSecond", "notStarted"},
},
Expand Down
Loading

0 comments on commit 1c67e77

Please sign in to comment.