Forcing DopplerSecret objects to be created in operator namespace breaks namespace isolation

[nickburgin]

Edit: This is partially my bad. I had a stale sealed secret obfuscating the real issue (see https://github.com/DopplerHQ/kubernetes-operator/issues/45#issuecomment-1641174859)

Original message (not overly relevant):

The following simple DopplerSecret definition leads to a kubernetes secret being created, but the keys in that secret are in snake_case rather than the documented default SCREAMING_SNAKE_CASE.

apiVersion: secrets.doppler.com/v1alpha1
kind: DopplerSecret
metadata:
  name: oauth2-secrets
spec:
  tokenSecret:
    name: doppler-service-token
  managedSecret:
    name: oauth2-secrets

I tried adding a nameTransformer with a value of upper-snake (which is not a documented supported value), but that leads to an error saying that it's not supported, advising that the supported values are supported values: "upper-camel", "camel", "lower-snake", "tf-var", "dotnet-env", "lower-kebab". None of these will get the keys back to the format they need to be in.

Unfortunately, this means that we can't make use of envFrom as our applications all expect env vars to be in SCREAMING_SNAKE_CASE. The workaround is to specify every env var with a valueFrom or change the implementation to use non-idiomatic env var styles.

Using the same project locally (outside of kubernetes) does not have this issue.

[watsonian]

@nickburgin The reason we don't have an upper-snake option is because that's the format ALL secrets in Doppler are stored in by default. You can't add a secret to Doppler without it being in upper-snake format currently. As such, when you create a managed secret, all the secret keys in that k8s secret will already be in upper-snake, so no transformation is required. Could you provide me with more information about what you're seeing that's different from this behavior?

[watsonian]

Also, as an aside, if you're using an up-to-date operator, then DopplerSecret CRDs need to be created in the operator namespace (doppler-operator-system by default) so you'd need to specify that namespace in the metadata section:

apiVersion: secrets.doppler.com/v1alpha1
kind: DopplerSecret
metadata:
  name: oauth2-secrets
  namespace: doppler-operator-system
spec:
  tokenSecret:
    name: doppler-service-token
  managedSecret:
    name: oauth2-secrets

The tokenSecret will look in the doppler-operator-system automatically by default if no namespace is specified and the managedSecret will be created in the default namespace by default if no namespace is specified. The tokenSecret and managedSecret can be in any namespace (as long as you specify it), but the CRD itself must exist in the same namespace as the operator currently and that's done by specifying the namespace in the metadata section as shown above.

[nickburgin]

After some further investigation, I realized that I had a stale sealedSecret object creating a secret with the same name. After cleaning that up, I am indeed observing that the DopplerSecret object in the target namespace (not doppler-operator-system) is not working (as you assert that it shouldn't above).

I have now created a working example by placing the DopplerSecret object in the doppler-operator-system namespace.

apiVersion: secrets.doppler.com/v1alpha1
kind: DopplerSecret
metadata:
  name: platform-demo-staging-oauth2-secrets # this is already pretty long, we have other examples while may be quite a bit longer
  namespace: doppler-operator-system # don't love it
spec:
  tokenSecret:
    name: doppler-service-token
    namespace: platform-demo-staging # if the dopper secret could be in any namespace, the default here should be the same namespace as where the doppler secret is located. I can understand wanting to default to the dopler-operator-system namespace when using a service account token though.
  managedSecret:
    name: oauth2-secrets
    namespace: platform-demo-staging

This is not how the behaviour is documented:

Note: While these resources can be created in any namespace, it is recommended that you create your Doppler Token Secret and DopplerSecret inside the doppler-operator-system namespace to prevent unauthorized access. The managed secret should be namespaced with the deployments which will use the secret.

Storing everything in the doppler-operator-system namespace would be a nightmare for us; we use a small number of clusters with a large number of deployed services in each cluster, each service is resident in it's own namespace, and quite a few of them will have overlapping secret names that will map to different doppler projects and/or configs. Is there a limitation on the length of a DopplerSecret name? We would also have to reduce some built in safety checks in our tooling to allow manifests to deploy objects to multiple namespaces, don't love the idea...

In this experiment, I have set up the terraform module that manages any given kubernetes namespace to provision a service token for the target doppler config, and store the token in a kubernetes secret object within that namespace, this feels better than provisioning and storing a single service account token for the whole cluster (which would have to have global read over all projects in our whole doppler workspace), which would be the obvious thing to do if all DopplerSecrets are to be stored in the same namespace.

Security wise in our clusters, the few people who can access the contents of that secret (or any secret in the namespace), can also get a shell in any pod in the namespace and inspect the environment directly, so there isn't any material advantage to hiding portions of that services config away in another namespace, just needless confusion and additional cognitive load on our developers. And the service token won't give them access to anything beyond what they could access anyway.

Further to this, I don't see how any part of the DopplerSecret could be considered sensitive enough to warrant placing it in a more controlled namespace than the token it requires or the secrets it manages, it's just some basic metadata.

Given how other kubernetes CRDs behave (contour, sealed-secrets and cert-manager to name a few that don't force objects to be created in a specific namespace), and the intent of the kubernetes developers (CRDs extend the behaviour of a cluster, and a namespace can be considered a virtual cluster - object definitions in a namespace can access that extended behavior through the newly defined resources, but should be able to remain isolated from other namespaces), it doesn't feel right (to me anyway) that Doppler is forcing certain non-idiomatic implementation details upon its users.

My suggestion would be to document your recommended approach (as you have done), but allow the people building cloud infrastructure to make their own decisions on how they manage security (which you have recently taken away by the sounds of it) as there are many ways to approach this, and the best option depends on the specific requirements of a particular team or organisation.

[watsonian]

@nickburgin We're planning on making a change that will make it so the operator can process DopplerSecret resources in any namespace. The only caveat will be that they'll have to use a tokenSecret and managedSecret that's in the same namespace as the DopplerSecret in those cases. The behavior as it exists today would also be possible. Would this work for you?

[watsonian]

Also, as an aside, I've updated the docs to clarify that the DopplerSecret must be in the same namespace as the operator (for now anyways).

[nickburgin]

@nickburgin We're planning on making a change that will make it so the operator can process DopplerSecret resources in any namespace. The only caveat will be that they'll have to use a tokenSecret and managedSecret that's in the same namespace as the DopplerSecret in those cases. The behavior as it exists today would also be possible. Would this work for you?

Yes, that would work perfectly