Merge pull request #224 from chmouel/create-delete-secret-clusterwide

config/200-role.yaml

@@ -82,10 +82,10 @@ rules:
   - apiGroups: [""]
     resources: ["namespaces", "pods", "pods/log"]
     verbs: ["get", "list", "watch"]
-  # Allow creating secrets in namespace
+  # Allow creating and deleting secrets in namespace
   - apiGroups: [""]
     resources: ["secrets"]
-    verbs: ["get", "create", "update"]
+    verbs: ["create", "delete"]
   # Permissions to list repositories on cluster
   - apiGroups: ["pipelinesascode.tekton.dev"]
     resources: ["repositories"]

pkg/cli/interface.go

@@ -49,5 +49,5 @@ type KubeInteractionIntf interface {
 	// TODO: we don't need tektonv1beta1client stuff here
 	WaitForPipelineRunSucceed(context.Context, tektonv1beta1client.TektonV1beta1Interface, *v1beta1.PipelineRun, time.Duration) error
 	CleanupPipelines(context.Context, string, string, int) error
-	CreateBasicAuthSecret(context.Context, webvcs.RunInfo, string, string) error
+	CreateBasicAuthSecret(context.Context, webvcs.RunInfo, string) error
 }

pkg/cmd/pipelineascode/pipelineascode.go

@@ -91,8 +91,7 @@ func parsePayload(ctx context.Context, cs *cli.Clients, opts *pacpkg.Options) (*
 		return nil, err
 	}
 
-	payloadinfo, err := cs.GithubClient.ParsePayload(ctx, cs.Log, opts.RunInfo.EventType,
-		opts.RunInfo.TriggerTarget, string(payloadB))
+	payloadinfo, err := cs.GithubClient.ParsePayload(ctx, cs.Log, opts.RunInfo, string(payloadB))
 	if err != nil {
 		return &webvcs.RunInfo{}, err
 	}

pkg/kubeinteraction/secrets.go

@@ -19,29 +19,38 @@ const (
 	`
 )
 
+func (k Interaction) createSecret(ctx context.Context, secretData map[string]string, targetNamespace, secretName string) error {
+	secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
+		Name:   secretName,
+		Labels: map[string]string{"app.kubernetes.io/managed-by": "pipelines-as-code"},
+	}}
+	secret.StringData = secretData
+	_, err := k.Clients.Kube.CoreV1().Secrets(targetNamespace).Create(ctx, secret, metav1.CreateOptions{})
+	return err
+}
+
 // CreateBasicAuthSecret Create a secret for git-clone basic-auth workspace
-func (k Interaction) CreateBasicAuthSecret(ctx context.Context, runinfo webvcs.RunInfo, targetNamespace, token string) error {
+func (k Interaction) CreateBasicAuthSecret(ctx context.Context, runinfo webvcs.RunInfo, targetNamespace string) error {
 	repoURL, err := url.Parse(runinfo.URL)
 	if err != nil {
 		return err
 	}
-	urlWithToken := fmt.Sprintf("%s://git:%s@%s%s", repoURL.Scheme, token, repoURL.Host, repoURL.Path)
-
+	urlWithToken := fmt.Sprintf("%s://git:%s@%s%s", repoURL.Scheme, k.Clients.GithubClient.Token, repoURL.Host, repoURL.Path)
 	secretData := map[string]string{
 		".gitconfig":       fmt.Sprintf(basicAuthGitConfigData, runinfo.URL),
 		".git-credentials": urlWithToken,
 	}
 
+	// Try to create secrete if that fails then delete it first and then create
+	// This allows up not to give List and Get right clusterwide
 	secretName := fmt.Sprintf(basicAuthSecretName, runinfo.Owner, runinfo.Repository)
-	secret, err := k.Clients.Kube.CoreV1().Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{})
+	err = k.createSecret(ctx, secretData, targetNamespace, secretName)
 	if err != nil {
-		secret = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName}}
-		secret.StringData = secretData
-		_, err = k.Clients.Kube.CoreV1().Secrets(targetNamespace).Create(ctx, secret, metav1.CreateOptions{})
-	} else {
-		secret.StringData = secretData
-		_, err = k.Clients.Kube.CoreV1().Secrets(targetNamespace).Update(ctx, secret, metav1.UpdateOptions{})
+		err = k.Clients.Kube.CoreV1().Secrets(targetNamespace).Delete(ctx, secretName, metav1.DeleteOptions{})
+		if err == nil {
+			err = k.createSecret(ctx, secretData, targetNamespace, secretName)
+		}
 	}
-
+	k.Clients.Log.Infof("Secret %s has been generated in namespace %s", secretName, targetNamespace)
 	return err
 }

pkg/kubeinteraction/secrets_test.go

@@ -6,6 +6,8 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/cli"
 	testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/webvcs"
+	"go.uber.org/zap"
+	zapobserver "go.uber.org/zap/zaptest/observer"
 	"gotest.tools/v3/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -50,9 +52,14 @@ func TestCreateBasicAuthSecret(t *testing.T) {
 		},
 	}
 	stdata, _ := testclient.SeedTestData(t, ctx, tdata)
-
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(observer).Sugar()
 	kint := Interaction{
-		Clients: &cli.Clients{Kube: stdata.Kube},
+		Clients: &cli.Clients{
+			Kube:         stdata.Kube,
+			GithubClient: webvcs.GithubVCS{Token: token},
+			Log:          fakelogger,
+		},
 	}
 	runinfo := webvcs.RunInfo{
 		Owner:      "owner",
@@ -75,10 +82,11 @@ func TestCreateBasicAuthSecret(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := kint.CreateBasicAuthSecret(ctx, runinfo, tt.targetNS, token)
+			err := kint.CreateBasicAuthSecret(ctx, runinfo, tt.targetNS)
 			assert.NilError(t, err)
 			slist, err := kint.Clients.Kube.CoreV1().Secrets(tt.targetNS).List(ctx, metav1.ListOptions{})
 			assert.NilError(t, err)
+			assert.Assert(t, len(slist.Items) > 0, "Secret has not been created")
 			assert.Equal(t, slist.Items[0].Name, "pac-git-basic-auth-owner-repo")
 			assert.Equal(t, slist.Items[0].StringData[".git-credentials"], "https://git:verysecrete@forge/owner/repo")
 		})

pkg/pipelineascode/pipelineascode.go

@@ -152,7 +152,7 @@ func Run(ctx context.Context, cs *cli.Clients, k8int cli.KubeInteractionIntf, ru
 
 	// Automatically create a secret with the token to be reused by git-clone task
 	if runinfo.SecretAutoCreation {
-		err = k8int.CreateBasicAuthSecret(ctx, *runinfo, repo.Spec.Namespace, cs.GithubClient.Token)
+		err = k8int.CreateBasicAuthSecret(ctx, *runinfo, repo.Spec.Namespace)
 		if err != nil {
 			return err
 		}
@@ -164,6 +164,7 @@ func Run(ctx context.Context, cs *cli.Clients, k8int cli.KubeInteractionIntf, ru
 	// be aware of it when querying.
 	refTomakeK8Happy := strings.ReplaceAll(runinfo.BaseBranch, "/", "-")
 	pipelineRun.Labels = map[string]string{
+		"app.kubernetes.io/managed-by":              "pipelines-as-code",
 		"pipelinesascode.tekton.dev/url-org":        runinfo.Owner,
 		"pipelinesascode.tekton.dev/url-repository": runinfo.Repository,
 		"pipelinesascode.tekton.dev/sha":            runinfo.SHA,

pkg/test/kubernetestint/kubernetesint.go

@@ -21,7 +21,7 @@ func (k *KinterfaceTest) GetConsoleUI(ctx context.Context, ns string, pr string)
 	return k.ConsoleURL, nil
 }
 
-func (k *KinterfaceTest) CreateBasicAuthSecret(ctx context.Context, runinfo webvcs.RunInfo, namespace, token string) error {
+func (k *KinterfaceTest) CreateBasicAuthSecret(ctx context.Context, runinfo webvcs.RunInfo, ns string) error {
 	return nil
 }
 

pkg/webvcs/github.go

@@ -188,10 +188,11 @@ func (v GithubVCS) populateCommitInfo(ctx context.Context, runinfo *RunInfo) err
 }
 
 // ParsePayload parse payload event
-func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, eventType, triggerTarget, payload string) (*RunInfo, error) {
+// TODO: this piece of code is just plain silly
+func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, optRunInfo RunInfo, payload string) (*RunInfo, error) {
 	var runinfo RunInfo
 	payload = payloadFix(payload)
-	event, err := github.ParseWebHook(eventType, []byte(payloadFix(payload)))
+	event, err := github.ParseWebHook(optRunInfo.EventType, []byte(payloadFix(payload)))
 	if err != nil {
 		return &runinfo, err
 	}
@@ -202,7 +203,7 @@ func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, eve
 
 	switch event := event.(type) {
 	case *github.CheckRunEvent:
-		if triggerTarget == "issue-recheck" {
+		if optRunInfo.TriggerTarget == "issue-recheck" {
 			runinfo, err = v.handleReRequestEvent(ctx, log, event)
 			if err != nil {
 				return &runinfo, err
@@ -224,7 +225,7 @@ func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, eve
 			SHATitle:      event.GetHeadCommit().GetMessage(),
 			Sender:        event.GetSender().GetLogin(),
 			BaseBranch:    event.GetRef(),
-			EventType:     eventType,
+			EventType:     optRunInfo.TriggerTarget,
 		}
 
 		runinfo.HeadBranch = runinfo.BaseBranch // in push events Head Branch is the same as Basebranch
@@ -238,7 +239,7 @@ func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, eve
 			BaseBranch:    event.GetPullRequest().Base.GetRef(),
 			HeadBranch:    event.GetPullRequest().Head.GetRef(),
 			Sender:        event.GetPullRequest().GetUser().GetLogin(),
-			EventType:     eventType,
+			EventType:     optRunInfo.EventType,
 		}
 	default:
 		return &runinfo, errors.New("this event is not supported")
@@ -250,7 +251,8 @@ func (v GithubVCS) ParsePayload(ctx context.Context, log *zap.SugaredLogger, eve
 	}
 
 	runinfo.Event = event
-	runinfo.TriggerTarget = triggerTarget
+	runinfo.TriggerTarget = optRunInfo.TriggerTarget
+	runinfo.SecretAutoCreation = optRunInfo.SecretAutoCreation
 	return &runinfo, nil
 }
 

pkg/webvcs/github_test.go

@@ -131,7 +131,11 @@ func TestPayLoadFix(t *testing.T) {
 	}
 
 	logger, _ := getLogger()
-	_, err = gvcs.ParsePayload(ctx, logger, "pull_request", "pull_request", string(b))
+	r := RunInfo{
+		EventType:     "pull_request",
+		TriggerTarget: "pull_request",
+	}
+	_, err = gvcs.ParsePayload(ctx, logger, r, string(b))
 	// would bomb out on "assertion failed: error is not nil: invalid character
 	// '\n' in string literal" if we don't fix the payload
 	assert.NilError(t, err)
@@ -164,8 +168,11 @@ func TestParsePayloadRerequestFromPullRequest(t *testing.T) {
 		Client: fakeclient,
 	}
 	logger, observer := getLogger()
-	// TODO
-	runinfo, err := gvcs.ParsePayload(ctx, logger, "check_run", "issue-recheck", checkrunEvent)
+	r := RunInfo{
+		EventType:     "check_run",
+		TriggerTarget: "issue-recheck",
+	}
+	runinfo, err := gvcs.ParsePayload(ctx, logger, r, checkrunEvent)
 	assert.NilError(t, err)
 
 	assert.Equal(t, prOwner, runinfo.Owner)
@@ -217,8 +224,12 @@ func TestParsePayloadRerequestFromPush(t *testing.T) {
 		_, _ = fmt.Fprint(w, `{"commit": {"message": "HELLO"}}`)
 	})
 
+	r := RunInfo{
+		EventType:     "check_run",
+		TriggerTarget: "issue-recheck",
+	}
 	logger, _ := getLogger()
-	runinfo, err := gvcs.ParsePayload(ctx, logger, "check_run", "issue-recheck", checkrunEvent)
+	runinfo, err := gvcs.ParsePayload(ctx, logger, r, checkrunEvent)
 	assert.NilError(t, err)
 
 	assert.Equal(t, runinfo.EventType, "push")
@@ -272,7 +283,11 @@ func TestParsePayLoadRetest(t *testing.T) {
 	}
 
 	// TODO
-	runinfo, err := gvcs.ParsePayload(ctx, logger, "issue_comment", "issue_comment", issueEvent)
+	r := RunInfo{
+		EventType:     "issue_comment",
+		TriggerTarget: "issue_comment",
+	}
+	runinfo, err := gvcs.ParsePayload(ctx, logger, r, issueEvent)
 	assert.NilError(t, err)
 	assert.Equal(t, prOwner, runinfo.Owner)
 	// Make sure the PR owner is the runinfo.Owner and not the issueSender
@@ -299,7 +314,11 @@ func TestParsePayload(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
 	logger, _ := getLogger()
 
-	runinfo, err := gvcs.ParsePayload(ctx, logger, "pull_request", "pull_request", string(b))
+	r := RunInfo{
+		EventType:     "pull_request",
+		TriggerTarget: "pull_request",
+	}
+	runinfo, err := gvcs.ParsePayload(ctx, logger, r, string(b))
 	assert.NilError(t, err)
 	assert.Assert(t, runinfo.BaseBranch == "master")
 	assert.Assert(t, runinfo.Owner == "chmouel")
@@ -312,23 +331,35 @@ func TestParsePayloadInvalid(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
 	gvcs := NewGithubVCS("none", "")
 	logger, _ := getLogger()
-	_, err := gvcs.ParsePayload(ctx, logger, "pull_request", "pull_request", "hello moto")
+	r := RunInfo{
+		EventType:     "pull_request",
+		TriggerTarget: "pull_request",
+	}
+	_, err := gvcs.ParsePayload(ctx, logger, r, "hello moto")
 	assert.ErrorContains(t, err, "invalid character")
 }
 
 func TestParsePayloadUnkownEvent(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
 	gvcs := NewGithubVCS("none", "")
 	logger, _ := getLogger()
-	_, err := gvcs.ParsePayload(ctx, logger, "foo", "foo", "{\"hello\": \"moto\"}")
+	r := RunInfo{
+		EventType:     "foo",
+		TriggerTarget: "foo",
+	}
+	_, err := gvcs.ParsePayload(ctx, logger, r, "{\"hello\": \"moto\"}")
 	assert.ErrorContains(t, err, "unknown X-Github-Event")
 }
 
 func TestParsePayCannotParse(t *testing.T) {
 	gvcs := NewGithubVCS("none", "")
 	ctx, _ := rtesting.SetupFakeContext(t)
 	logger, _ := getLogger()
-	_, err := gvcs.ParsePayload(ctx, logger, "gollum", "gollum", "{}")
+	r := RunInfo{
+		EventType:     "gollum",
+		TriggerTarget: "gollum",
+	}
+	_, err := gvcs.ParsePayload(ctx, logger, r, "{}")
 	assert.Error(t, err, "this event is not supported")
 }