tektoncd / chains

Supply Chain Security in Tekton Pipelines
Apache License 2.0
246 stars 129 forks source link

Support for Mongo Token rotation #1089

Closed PuneetPunamiya closed 2 months ago

PuneetPunamiya commented 6 months ago

Feature request

As of today to store the attestations to mongodb we need to set MONGO_SERVER_URL as an env to the chains controller. For ref: https://github.com/google/go-cloud/blob/master/docstore/mongodocstore/urls.go#L42-L60

But if the mongo token is rotated then as of today we again need to set the MONGO_SERVER_URL value so that chains controller uses the new value

Hence in order to get the rotated token values, there can be 2 ways

In bot the cases, the common is reading mongo server url from a path

lcarva commented 6 months ago

I'm not too familiar with MongDB, but it sounds like the URL also includes sensitive information, like a token.

If that's the case, I would prefer to not store that in the Chains config for the same reason as #1074.

IIRC, from the KMS case, we talked about adding a new key to the Chains config, e.g. signers.kms.auth.token-path. Could we take the same approach here? Introduce a new Chains config key, e.g. storage.mogodb.url-path, to point to the file where the MongoDB Server URL is set?

concaf commented 6 months ago

I'm not too familiar with MongDB, but it sounds like the URL also includes sensitive information, like a token.

you're right, it contains creds to connect to mongoDB.

IIRC, from the KMS case, we talked about adding a new key to the Chains config, e.g. signers.kms.auth.token-path. Could we take the same approach here? Introduce a new Chains config key, e.g. storage.mogodb.url-path, to point to the file where the MongoDB Server URL is set?

i agree with this approach - reading from a path is more ... inclusive than reading from a secret due to the following use cases:

another thing to note is that MONGO_SERVER_URL is not parsed by the chains controller, instead it's read from env by google/go-cloud - which means that MONGO_SERVER_URL needs to be generated on the fly from signers.kms.auth.token-path by chains controller so google/go-cloud can read the up to date value this also eliminates the need to restart the pod to read the updated value

concaf commented 6 months ago

/assign

concaf commented 6 months ago

@lcarva to configure mongo as a storage backend, i believe there are 2 things that need to be done:

how about these as new fields?

lcarva commented 6 months ago

Sure. We can sort out the exact names in the pull request :wink:

concaf commented 5 months ago

@lcarva when is the next chains release? i want this to be a part of the next release 🤔

lcarva commented 5 months ago

We aim for a release every month. But, usually, we go longer than that.