Let the user pass a secret for on git API resolver

We were previously only allowing using a secret from the global configmap, which mean every users on the cluster would have to use the same git value (which could be a security issue). We now let the user use their own token on their own namespace with the optional "token" and (even more optional) "tokenKey" params. The fallback will still be the global configmap. Added a bit logic to make sure we are caching secrets only when user use the token from global configmap. Since we can't ensure for local secrets on namespace may be rotated often. Signed-off-by: Chmouel Boudjnah <[email protected]>
tektoncd · Oct 27, 2023 · 1025878 · 1025878
1 parent 43ac007
commit 1025878
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 34 deletions.
diff --git a/docs/git-resolver.md b/docs/git-resolver.md
@@ -18,8 +18,10 @@ This Resolver responds to type `git`.
 | `url`        | URL of the repo to fetch and clone anonymously. Either `url`, or `repo` (with `org`) must be specified, but not both.  | `https://github.com/tektoncd/catalog.git`                   |
 | `repo`       | The repository to find the resource in. Either `url`, or `repo` (with `org`) must be specified, but not both.          | `pipeline`, `test-infra`                                    |
 | `org`        | The organization to find the repository in. Default can be set in [configuration](#configuration).                     | `tektoncd`, `kubernetes`                                    |
+| `token`      | An optional secret name in the `PipelineRun` namespace to fetch the token from. Defaults to empty, meaning it will try to use the configuration from the global configmap.      | `secret-name`, (empty)  |
+| `tokenKey`   | An optional key in the token secret name in the `PipelineRun` namespace to fetch the token from. Defaults to `token`.  | `token`                                                     |
 | `revision`   | Git revision to checkout a file from. This can be commit SHA, branch or tag.                                           | `aeb957601cf41c012be462827053a21a420befca` `main` `v0.38.2` |
-| `pathInRepo` | Where to find the file in the repo.                                                                                    | `task/golang-build/0.3/golang-build.yaml`                  |
+| `pathInRepo` | Where to find the file in the repo.                                                                                    | `task/golang-build/0.3/golang-build.yaml`                   |
 
 ## Requirements
 
@@ -158,8 +160,8 @@ spec:
 ## `ResolutionRequest` Status
 `ResolutionRequest.Status.RefSource` field captures the source where the remote resource came from. It includes the 3 subfields: `url`, `digest` and `entrypoint`.
 - `url`
-  - If users choose to use anonymous cloning, the url is just user-provided value for the `url` param in the [SPDX download format](https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field). 
-  - If scm api is used, it would be the clone URL of the repo fetched from scm repository service in the [SPDX download format](https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field). 
+  - If users choose to use anonymous cloning, the url is just user-provided value for the `url` param in the [SPDX download format](https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field).
+  - If scm api is used, it would be the clone URL of the repo fetched from scm repository service in the [SPDX download format](https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field).
 - `digest`
   - The algorithm name is fixed "sha1", but subject to be changed to "sha256" once Git eventually uses SHA256 at some point later. See https://git-scm.com/docs/hash-function-transition for more details.
   - The value is the actual commit sha at the moment of resolving the resource even if a user provides a tag/branch name for the param `revision`.
@@ -189,7 +191,7 @@ spec:
 apiVersion: resolution.tekton.dev/v1alpha1
 kind: ResolutionRequest
 metadata:
-  ... 
+  ...
 spec:
   params:
     pathInRepo: pipeline.yaml

diff --git a/pkg/resolution/resolver/git/params.go b/pkg/resolution/resolver/git/params.go
@@ -27,4 +27,10 @@ const (
 	pathParam string = "pathInRepo"
 	// revisionParam is the git revision that a file should be fetched from. This is used with both approaches.
 	revisionParam string = "revision"
+	// tokenParam is an optional reference to a secret name for SCM API authentication
+	tokenParam string = "token"
+	// tokenKeyParam is an optional reference to a key in the tokenParam secret for SCM API authentication
+	tokenKeyParam string = "tokenKey"
+	// defaultTokenKeyParam is the default key in the tokenParam secret for SCM API authentication
+	defaultTokenKeyParam string = "token"
 )
diff --git a/pkg/resolution/resolver/git/resolver.go b/pkg/resolution/resolver/git/resolver.go
@@ -35,6 +35,7 @@ import (
 	"github.com/jenkins-x/go-scm/scm/factory"
 	resolverconfig "github.com/tektoncd/pipeline/pkg/apis/config/resolver"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
+	"github.com/tektoncd/pipeline/pkg/resolution/common"
 	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
 	"github.com/tektoncd/pipeline/pkg/resolution/resolver/framework"
 	"go.uber.org/zap"
@@ -147,7 +148,19 @@ func (r *Resolver) resolveAPIGit(ctx context.Context, params map[string]string)
 	if err != nil {
 		return nil, err
 	}
-	apiToken, err := r.getAPIToken(ctx)
+	secretRef := &secretCacheKey{
+		name: params[tokenParam],
+		key:  params[tokenKeyParam],
+	}
+	if secretRef.name != "" {
+		if secretRef.key == "" {
+			secretRef.key = defaultTokenKeyParam
+		}
+		secretRef.ns = common.RequestNamespace(ctx)
+	} else {
+		secretRef = nil
+	}
+	apiToken, err := r.getAPIToken(ctx, secretRef)
 	if err != nil {
 		return nil, err
 	}
@@ -366,51 +379,66 @@ func (r *Resolver) getSCMTypeAndServerURL(ctx context.Context) (string, string,
 	return scmType, serverURL, nil
 }
 
-func (r *Resolver) getAPIToken(ctx context.Context) ([]byte, error) {
+func (r *Resolver) getAPIToken(ctx context.Context, apiSecret *secretCacheKey) ([]byte, error) {
 	conf := framework.GetResolverConfigFromContext(ctx)
 
-	cacheKey := secretCacheKey{}
-
 	ok := false
 
-	if cacheKey.name, ok = conf[APISecretNameKey]; !ok || cacheKey.name == "" {
-		err := fmt.Errorf("cannot get API token, required when specifying '%s' param, '%s' not specified in config", repoParam, APISecretNameKey)
-		r.logger.Info(err)
-		return nil, err
+	// NOTE(chmouel): only cache secrets when user hasn't passed params in their resolver configuration
+	cacheSecret := false
+	if apiSecret == nil {
+		cacheSecret = true
+		apiSecret = &secretCacheKey{}
 	}
-	if cacheKey.key, ok = conf[APISecretKeyKey]; !ok || cacheKey.key == "" {
-		err := fmt.Errorf("cannot get API token, required when specifying '%s' param, '%s' not specified in config", repoParam, APISecretKeyKey)
-		r.logger.Info(err)
-		return nil, err
+
+	if apiSecret.name == "" {
+		if apiSecret.name, ok = conf[APISecretNameKey]; !ok || apiSecret.name == "" {
+			err := fmt.Errorf("cannot get API token, required when specifying '%s' param, '%s' not specified in config", repoParam, APISecretNameKey)
+			r.logger.Info(err)
+			return nil, err
+		}
 	}
-	if cacheKey.ns, ok = conf[APISecretNamespaceKey]; !ok {
-		cacheKey.ns = os.Getenv("SYSTEM_NAMESPACE")
+	if apiSecret.key == "" {
+		if apiSecret.key, ok = conf[APISecretKeyKey]; !ok || apiSecret.key == "" {
+			err := fmt.Errorf("cannot get API token, required when specifying '%s' param, '%s' not specified in config", repoParam, APISecretKeyKey)
+			r.logger.Info(err)
+			return nil, err
+		}
+	}
+	if apiSecret.ns == "" {
+		if apiSecret.ns, ok = conf[APISecretNamespaceKey]; !ok {
+			apiSecret.ns = os.Getenv("SYSTEM_NAMESPACE")
+		}
 	}
 
-	val, ok := r.cache.Get(cacheKey)
-	if ok {
-		return val.([]byte), nil
+	if cacheSecret {
+		val, ok := r.cache.Get(apiSecret)
+		if ok {
+			return val.([]byte), nil
+		}
 	}
 
-	secret, err := r.kubeClient.CoreV1().Secrets(cacheKey.ns).Get(ctx, cacheKey.name, metav1.GetOptions{})
+	secret, err := r.kubeClient.CoreV1().Secrets(apiSecret.ns).Get(ctx, apiSecret.name, metav1.GetOptions{})
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			notFoundErr := fmt.Errorf("cannot get API token, secret %s not found in namespace %s", cacheKey.name, cacheKey.ns)
+			notFoundErr := fmt.Errorf("cannot get API token, secret %s not found in namespace %s", apiSecret.name, apiSecret.ns)
 			r.logger.Info(notFoundErr)
 			return nil, notFoundErr
 		}
-		wrappedErr := fmt.Errorf("error reading API token from secret %s in namespace %s: %w", cacheKey.name, cacheKey.ns, err)
+		wrappedErr := fmt.Errorf("error reading API token from secret %s in namespace %s: %w", apiSecret.name, apiSecret.ns, err)
 		r.logger.Info(wrappedErr)
 		return nil, wrappedErr
 	}
 
-	secretVal, ok := secret.Data[cacheKey.key]
+	secretVal, ok := secret.Data[apiSecret.key]
 	if !ok {
-		err := fmt.Errorf("cannot get API token, key %s not found in secret %s in namespace %s", cacheKey.key, cacheKey.name, cacheKey.ns)
+		err := fmt.Errorf("cannot get API token, key %s not found in secret %s in namespace %s", apiSecret.key, apiSecret.name, apiSecret.ns)
 		r.logger.Info(err)
 		return nil, err
 	}
-	r.cache.Add(cacheKey, secretVal, r.ttl)
+	if cacheSecret {
+		r.cache.Add(apiSecret, secretVal, r.ttl)
+	}
 	return secretVal, nil
 }
 

diff --git a/pkg/resolution/resolver/git/resolver_test.go b/pkg/resolution/resolver/git/resolver_test.go
@@ -192,10 +192,13 @@ type params struct {
 	pathInRepo string
 	org        string
 	repo       string
+	token      string
+	tokenKey   string
+	namespace  string
 }
 
 func TestResolve(t *testing.T) {
-	// lcoal repo set up for anonymous cloning
+	// local repo set up for anonymous cloning
 	// ----
 	commits := []commitForRepo{{
 		Dir:      "foo/",
@@ -327,6 +330,24 @@ func TestResolve(t *testing.T) {
 			url:        anonFakeRepoURL,
 		},
 		expectedErr: createError("revision error: reference not found"),
+	}, {
+		name: "api: successful task from params api information",
+		args: &params{
+			revision:   "main",
+			pathInRepo: "tasks/example-task.yaml",
+			org:        testOrg,
+			repo:       testRepo,
+			token:      "token-secret",
+			tokenKey:   "token",
+			namespace:  "foo",
+		},
+		config: map[string]string{
+			ServerURLKey: "fake",
+			SCMTypeKey:   "fake",
+		},
+		apiToken:          "some-token",
+		expectedCommitSHA: commitSHAsInSCMRepo[0],
+		expectedStatus:    internal.CreateResolutionRequestStatusWithData(mainTaskYAML),
 	}, {
 		name: "api: successful task",
 		args: &params{
@@ -539,21 +560,27 @@ func TestResolve(t *testing.T) {
 			}
 
 			frtesting.RunResolverReconcileTest(ctx, t, d, resolver, request, expectedStatus, tc.expectedErr, func(resolver framework.Resolver, testAssets test.Assets) {
-				if tc.config[APISecretNameKey] == "" || tc.config[APISecretNamespaceKey] == "" || tc.config[APISecretKeyKey] == "" || tc.apiToken == "" {
+				var secretName, secretNameKey, secretNamespace string
+				if tc.config[APISecretNameKey] != "" && tc.config[APISecretNamespaceKey] != "" && tc.config[APISecretKeyKey] != "" && tc.apiToken != "" {
+					secretName, secretNameKey, secretNamespace = tc.config[APISecretNameKey], tc.config[APISecretKeyKey], tc.config[APISecretNamespaceKey]
+				}
+				if tc.args.token != "" && tc.args.namespace != "" && tc.args.tokenKey != "" {
+					secretName, secretNameKey, secretNamespace = tc.args.token, tc.args.tokenKey, tc.args.namespace
+				}
+				if secretName == "" || secretNameKey == "" || secretNamespace == "" {
 					return
 				}
 				tokenSecret := &corev1.Secret{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      tc.config[APISecretNameKey],
-						Namespace: tc.config[APISecretNamespaceKey],
+						Name:      secretName,
+						Namespace: secretNamespace,
 					},
 					Data: map[string][]byte{
-						tc.config[APISecretKeyKey]: []byte(base64.StdEncoding.Strict().EncodeToString([]byte(tc.apiToken))),
+						secretNameKey: []byte(base64.StdEncoding.Strict().EncodeToString([]byte(tc.apiToken))),
 					},
 					Type: corev1.SecretTypeOpaque,
 				}
-
-				if _, err := testAssets.Clients.Kube.CoreV1().Secrets(tc.config[APISecretNamespaceKey]).Create(ctx, tokenSecret, metav1.CreateOptions{}); err != nil {
+				if _, err := testAssets.Clients.Kube.CoreV1().Secrets(secretNamespace).Create(ctx, tokenSecret, metav1.CreateOptions{}); err != nil {
 					t.Fatalf("failed to create test token secret: %v", err)
 				}
 			})
@@ -734,6 +761,16 @@ func createRequest(args *params) *v1beta1.ResolutionRequest {
 			Name:  orgParam,
 			Value: *pipelinev1.NewStructuredValues(args.org),
 		})
+		if args.token != "" {
+			rr.Spec.Params = append(rr.Spec.Params, pipelinev1.Param{
+				Name:  tokenParam,
+				Value: *pipelinev1.NewStructuredValues(args.token),
+			})
+			rr.Spec.Params = append(rr.Spec.Params, pipelinev1.Param{
+				Name:  tokenKeyParam,
+				Value: *pipelinev1.NewStructuredValues(args.tokenKey),
+			})
+		}
 	}
 
 	return rr