Currently, there is no way to make secrets available to an auto devops application running in a k8s cluster.
Further details
For example I may need to configure my rails application to have some secret API token for a 3rd party email sending service. I would prefer not to commit this secret to the repo as this violates 12 factor principles as well as probably violates some security best practices.
Maybe phrased as a user user story:
As an operator I'd like to be able to configure a deployed application with secret variables without this being visible to developers that write the code.
Possibly we need a new interface for this or maybe we can leverage our CI secret variables.
Proposal
Update documentation to reflect new prefixed variable option and link within the UI. Prefix with K8S_SECRET_
Update descriptive text within the UI:
Variables
Variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use variables for things like passwords, secret keys, or credentials. You may also add variables that are made available to the running application. More information
In CI we ensure the variables with the relevant prefix name are passed to the running app as env vars with the prefix stripped off
Previous Proposal 01
Proposal
Leveraging existing variables (project settings >> CI/CD >> variables) allow user to define these variables in a new section, where the variables are only to be used for app running in the cluster (the auto devops app).
We store these new variables in a new model in our backend (really simple new table)
When created we create the Secret in K8s
We pass a new variable to CI jobs called APP_SECRETS which is a comma separated list of the variables (names only) that need to be mounted in the helm chart
ce
ee
Only variables defined in the new section would be passed to the application running in the cluster.
Previous Proposal 02
These can then be configured for the application in a couple of ways:
What does success look like, and how can we measure that?
(If no way to measure success, link to an issue that will implement a way to measure this)
Links / references
This page may contain information related to upcoming products, features and functionality.
It is important to note that the information presented is for informational purposes only, so please do not rely on the information for purchasing or planning purposes.
Just like with all projects, the items mentioned on the page are subject to change or delay, and the development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.
isnt this already possible with CI / CD Variables ?(Settings -> CI / CD -> Variables)
@aradiv I don't think these variables will be made available to the running application on K8s. They will only be available during the build pipeline phase.
Dylan Griffithchanged title from Provide some way for users to configure their auto devops deployed applications with secrets that aren't committed to the repo to Provide some way for users to configure their Auto DevOps deployed applications with secrets that aren't committed to the repo
changed title from Provide some way for users to configure their auto devops deployed applications with secrets that aren't committed to the repo to Provide some way for users to configure their Auto DevOps deployed applications with secrets that aren't committed to the repo
Dylan Griffithchanged title from Provide some way for users to configure their Auto DevOps deployed applications with secrets that aren't committed to the repo to Provide some way configure Auto DevOps deployed applications with secrets that aren't committed to the repo
changed title from Provide some way for users to configure their Auto DevOps deployed applications with secrets that aren't committed to the repo to Provide some way configure Auto DevOps deployed applications with secrets that aren't committed to the repo
Dylan Griffithchanged title from Provide some way configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Provide some way to configure Auto DevOps deployed applications with secrets that aren't committed to the repo
changed title from Provide some way configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Provide some way to configure Auto DevOps deployed applications with secrets that aren't committed to the repo
I don't think these variables will be made available to the running application on K8s. They will only be available during the build pipeline phase.
@DylanGriffith We could, of course, modify the Auto DevOps scripts to pass more variables to Kubernetes. You can’t share all variables with Kubernetes because the job can’t tell the difference between variables you declared and variables set by the system, like PATH, SHELL, etc. Passing those variables could have horrible consequences. The runner might even know the difference, but doesn’t pass this information to the shell, afaik. If we had some kind of list of project (and group) variables separate from the environment, we could pass all of those to k8s and greatly simplify things. I'm sure then people will complain about creds being passed, which is a valid concern.
@markpundsack yeah I think it's risky if we're passing all these project and group variables through to the running app. I also think this will make the experience very confusing for users not distinguishing between these environments. Anyway I think the best options are to either have explicit UX for configuring these variables OR use some kind of convention approach like K8_ prefix that was suggested in the related issues.
I plan to actually test this out myself and provide more detailed instructions in future so that will help more people before we're able to actually implement this feature.
Though for those that are requesting this feature it would be really good to know how they would like it to work from a user perspective. This is one of the open questions we have.
These variables are specific to operations and might be different for each environment.
So I think they should be configured under operations -> environments.
Maybe have a link to it from the Settings -> CI/CD -> Variables (some users will probably look there).
Alternatively it could be placed under CI/CD -> Variables, but it would need to be possible to set a different values for the same variable in different environments.
These variables are specific to operations and might be different for each environment. So I think they should be configured under operations -> environments
@rp1 I am inclined to agree with your approach to keep them under Operations menu and separate from CI variables.
I believe that users should really not be burdened with the mental overhead that can happens when convoluting CI variables with application variables so we should really keep them seperate in our UI to make it easier for our users.
Disregarding where the values come from, using the raw value to pass to environment of a running pod is not a safe practice. You're best of creating a persistent Secret that contains one or more secure values, and then passing these as environment at container instantiation, with the definitions as such:
@adamlem this is awesome! We really love this kind of engagement and support from our community and this is a really practical solution that can help guide us to something we know will work for our users. Thank you so much. Please keep engaging and sharing these useful solutions on our issues!
I noticed you're using CI variables with specialised names and this is an option we've considered. We're also considering having a separate UI to configure these variables. I see that you want to configure 2 types:
ConfigMap
Secret
Would a separate UI be helpful to you?
Would you be able to share what you'd expect to see from a user perspective in GitLab to help you manage these secrets better?
Would you like a way to specify in the UI whether to mount as file OR env var and specify path and env var name as appropriate?
Do you have any gotchas you discovered along the way when setting this up?
Just trying to give something back, since I'm benefiting so much from the auto devops development from you guys.
I'm sure an environment(aka job stage)-focused view(and edit) via Operations -> Environments can help especially with a growing number of keys. I don't think it needs to be an OR. It could be a simplified view and edit via Operations -> Environments which adds to Settings -> CI/CD -> Variables with the right environment/stage prefix, type(ConfigMap/Secret) and real key.
I prefer to specify environment variables instead over mounting/reading files but only because I want to have only one way to specify configurations for our applications. A lot of frameworks directly support environment variables without extra work for the developers.
Mounting is not needed for our cases so far and I'm sure it is quite ugly to find good pattern if it should fit as normal CI/CD variable.
I had quite some trouble getting the parsing helper script running since it is embedded into the .auto_devops: &auto_devops | template. Should be easy for you guys to extract, rewrite and use it more like a service.
This is helpful feedback. This tells me we can start offering value to users without implementing file mounting. This can happen in a later iteration if customers need this.
I'm sure an environment(aka job stage)-focused view(and edit) via Operations -> Environments can help especially with a growing number of keys
This is good to know. We'll probably end up implementing this. I'm not sure if it will necessarily follow the same env var prefixing approach you used but this may well be a way we can accomplish it.
Alternatively we may prefer to actually create these secrets and configmaps via our backend so that they are never exposed to the runner during CI. Do you think this would cause problems for your use case?
I don't see any problem with that, I would be happy to get rid of my small bash hack :)
In my case I define review_configmaps_app_KEY(or secrets) for all review environments together which is quite handy since every branch has it own environment and adding it for each branch would be quite painful.
Just trying to describe the full pattern:
[production|staging|review]_[configmaps|secrets}_[configmapName|secretName]_Key
While for now I just use app as configmapName/secretName. (I'm sure it is not hard to loop over this keys in the helm chart too). For now one configMap/secret is enough in our cases.
Considering this feature is full stack involving frontend, backend and UX I propose that we do this in the following order (perhaps as a little experiment):
Work together on the basic UX and get mockups
frontend and backend work together to define the API needed to support the new UX
frontend and backend work in parallel based on the API specification
if backend finishes first then just merge the backend work (it should just be new APIs and such)
if frontend finishes first then merge the work behind a feature flag and ensure the feature flag is removed when backend is merged
@DylanGriffith@danielgruesso FWIW, At Heroku, we used to debate whether we needed to differentiate between compile-time and run-time variables. i.e. variables that the buildpack needed during the compile phase, and variables that the running process needed. We ended up deciding not to create separate pools of variables, and just make all variables available in both phases.
Likewise, Unix environment variables "leak" content to every process that runs, but no one seems to complain.
Having said that, I think we do want to differentiate. Our stack is a little more complex. There's various jobs that each may need different variables. e.g. running CI doesn't need access to the k8s creds; only the deploy job needs that. But then even within deploy, there's variables for the job itself, then variables for Helm, then variables for k8s/docker. And there's additional concerns about somehow providing credentials without letting the CI/CD job have access at all.
Operations -> Environments is interesting, but might be confusing with environment-specific variables.
I'm obviously biased, but https://gitlab.com/gitlab-org/gitlab-ce/issues/46806 seems to be a good general approach. By following conventions, variables can be passed to the parts as needed. e.g. the CI/CD job looks for variables matching a convention and passes them to Helm and k8s as appropriate. But, this puts all the complexity into the CI/CD job, which is great, frankly, and easy to iterate on. Having GitLab put the variables directly into k8s secrets/config is an interesting alternative (or iteration), but with some cons (separate structure, only works for k8s, likely more effort and less MVC).
One interesting challenge with putting things in k8s secrets/config is that people will expect to be able to edit them directly via kubectl (or other k8s admin). And in fact, this is a great opportunity for us; to be k8s native, and use k8s as the primary data store. But this is a huge departure for us. And in the absence of embracing it, we'll end up with weird situations where someone edits a k8s value directly, and then on the next deploy we overwrite it. That already happens with environment scaling.
Daniel Gruessochanged title from Provide some way to configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Configure Auto DevOps deployed applications with secrets that aren't committed to the repo
changed title from Provide some way to configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Configure Auto DevOps deployed applications with secrets that aren't committed to the repo
Having GitLab put the variables directly into k8s secrets/config is an interesting alternative (or iteration), but with some cons (separate structure, only works for k8s, likely more effort and less MVC)
@markpundsack I think both approaches "only work for k8s". Not of course the CI approach could be adapted so that another deploy script for non-k8s also read these variables but this is still all implicit anyway. We haven't really built a generic feature initially we've just built a UI on top of our existing variables and still needs code added for each platform to utilise it. Anyway interesting your above point considering I had not actually considered implementing it this way. I was pretty much always imagining that CI would do some of the work for us anyway but now that you mentioned it I'm possibly considering the idea of going straight to K8s to create the data.
One interesting challenge with putting things in k8s secrets/config is that people will expect to be able to edit them directly via kubectl (or other k8s admin). And in fact, this is a great opportunity for us; to be k8s native, and use k8s as the primary data store. But this is a huge departure for us. And in the absence of embracing it, we'll end up with weird situations where someone edits a k8s value directly, and then on the next deploy we overwrite it. That already happens with environment scaling.
@markpundsack this is a really interesting point and one I had not considered. How would you feel about attempting to "embrace" this somehow in our first MVC here. I can think of ways we can do this but they are all going to come with challenges and like you said it is a huge departure. But with that said should we try and brainstorm some approaches here and see what we can do? Or would you rather us just take the road well travelled and stick with doing everything on the GitLab side?
Ultimately I'm not strongly inclined to go either way CI variables or directly to K8s as data store but I am inclined to not paint ourselves into a corner here. If we have a long term vision that relates to this work I'd at least like to consider how we can ensure we're taking a step towards that long term vision while doing this feature. If we don't really have a long term vision and think that the CI approach is just as optimal as the k8s approach then maybe we should just do the easier one (probably CI).
Something else I just considered (while comparing this to scaling) is that when we couple simple config (or things that are easy to do the k8s way) to our CI infrastructure we create a problem where something that used to be just a single command that took effect very quickly now becomes a confusing UX where a user edits something which then triggers a pipeline and eventually takes effect after the pipeline either succeeds OR fails in which case you are debugging a bunch of strange commands that triggered. We can always hide this UX from the user but the complexity will still exist. As we build out more of ~Configure I'm starting to think we want to start to think a little about what our vision is for UX here because we're coupling ourselves more and more to CI jobs with every piece of UI (secrets, scaling, domain names) we build on top of env variables that are passed to CI.
I think doing it in k8s will be slower and provide less portability, that said, I'm not opposed to it. What's the boring solution here? (CI from my pov).
I'm not sure either solution is more boring than any other. We already have code which calls APIs in K8s and we also have code that sets env variables for CI. My biggest problem with the CI approach is that this will mean that we need to trigger a whole pipeline to run every time we change variables. I wonder how well this will scale with our longer term visions for application control panel?
@tauriedavis from a UX standpoint and depending on the engineering decision, if the secrets are to be used in running applications for auto devops, that means A) a cluster is present and B) auto devops is enabled for the project. If that's the case then the user can use this option (otherwise if we present the option but user cannot use because the necessary components are not in place it could cause confusion). This feels kinda like domain, which belongs to both k8s and auto devops and we could easily place in the wrong place.
Yes, of course. I didn't give much attention to be consistence in the examples since I just wanted to share the concept. I will try to be more correct in future :)
Yes, of course. Paths a predefined but customize-able for helm charts, I use chart/template/secrets.yaml & chart/template/configmaps.yaml
Done, just too limited. I'm maintaining my own version based on the autodevops gitlab ci template, I've got more customization in place since our tech stack is not fully covered by GitLabs auto devops.
In some of the comments it seems like it would be the K8s secrets, but the only interesting secrets are those actually being used in the environment of the pod.
If it were to be backed by K8s directly, it would be the environment settings of the application pod, right? Not, at least initially, environment settings for the Postgres and other service pods.
If you were to add a new variable then the pod would need to be restarted and have the environment variable added in its deployment.yaml or similar (I only have a little experience with K8s). On the next CI/CD run, it should know about this change and include the added variable (also remember that a CI/CD run could be running at the time when you add a variable).
@DylanGriffith Creating variables that the CI/CD job passes on to the app could support non-k8s easily as well.
If we have a long term vision that relates to this work I'd at least like to consider how we can ensure we're taking a step towards that long term vision while doing this feature. If we don't really have a long term vision and think that the CI approach is just as optimal as the k8s approach then maybe we should just do the easier one (probably CI).
Long term, we need to have k8s-native solutions for at least some parts. e.g. embrace Application CRD and other CRDs as they're standardized. Embrace Operators. Embrace Kubernetes as the data store (https://gitlab.com/gitlab-org/gitlab-ce/issues/47998).
But do not let a better long-term get in the way of small iterations now. I imagine it would be easier to make an MVC that took some convention of variable naming, and selectively presented the data via Helm charts to the running pods. Not positive about that, of course, so feel free to counter. The example above looks like a good start.
Creating variables that the CI/CD job passes on to the app could support non-k8s easily as well.
Yeah but the point I was making is that both approaches require us to write more code to support another platform. I guess you might say that basing on named CI/CD variables may seem a little easier.
Right now we don't actually have a free backend developer to start on this just yet so I'd much rather focus on understanding UX first since I think the backend implementation could be done either way. Also even if we end up building something really simple in the first release it would still have been valuable for us to see where we'd like the UX to get to in a for more iterations.
@tauriedavis@DylanGriffith we already have a way to create variables, so the boring solution is to leverage that. Of course we don't want to pass all the variables to the app so we need a way to specify which ones to pass. In my mind that would look something like this:
This gives us a way to even scope it to environment which is good.
We would need a way for the user to indicate that this variable is for use in the auto devops app only, which in the above example is represented by the checkbox. We can add a what's this? link to docs explaining what it is.
Do I understand it correct that the "Auto DevOps Apps Only"-checkbox will be responsible to define to put the variables in a Kubernetes secret or not? And by choosing the environment it will ensure that this Kubernetes secret exposes the variables to the right pod?
As long as "All environments" can be overwritten by a more specific environment I guess our case is fully covered by this solution.
Do I understand it correct that the "Auto DevOps Apps Only"-checkbox will be responsible to define to put the variables in a Kubernetes secret or not? And by choosing the environment it will ensure that this Kubernetes secret exposes the variables to the right pod?
As long as "All environments" can be overwritten by a more specific environment I guess our case is fully covered by this solution.
@adamlem yeah for sure the intention will be that whatever we do we can expose this variable to the app as an env var. And we will probably also ensure that we can set this on a per environment basis considering this is going to be pretty much essential for most users.
@danielgruesso I'm wondering about the fact that secret variables only allow environment scope in GitLab premium and above. From my perspective this would be pretty difficult for anybody on Core to use this feature as most often people want different credentials in staging and production. I think maybe they could find some ways to workaround it but I'm just thinking that considering we support staging and production in Auto DevOps on core then probably we should also have some way for users to configure different secrets for these environments.
We could always start with the current proposal and see how users react and if there are going to be lots of people asking for environment scope then we may need to change the permissions but this is a possible downside to tying the UX in with CI variables since we may end up wanting different licensing rules.
We could always start with the current proposal and see how users react and if there are going to be lots of people asking for environment scope then we may need to change the permissions
I'm all for this, let's let the env scope earn its way into the app.
Another possibility would be to reveal an env textbox only when the user checks the box for auto devops only.
From my understanding, @danielgruesso's proposal combines compile-time and run-time variables.
If a user needs to revoke a key/value (say it got leaked) and replace it with a new one, then the pipeline would need to fully run in order for that to take effect. Is the thought that we would change the functionality here to always run the pipeline against the last SHA that was deployed to the scoped environment (or all environments in the case of CE currently) when you add or edit a variable? It's a little awkward since I think the expectation is that the pipeline runs when you push code. A simple solution is to just inform the user that the pipeline is running and the changes will take effect when it completes. A little strange but works.
We could also look at creating two separate sections under variables: compile-time variables and run-time variables. We could always include environment scoping, even in CE, for run-time variables. They would always be available to apps in k8s. Does the entire pipeline need to run? I think only deploy matters in this case but what are the ramifications there? We don't have any sort of UI today that runs just 1 job of a pipeline, right?
In my case I need to re-run the deployment job(only) AND delete the related pod(s) otherwise they don't pick up the new values from the configmap/secret. Would be awesome if you guys could ensure that a change in the values will update the secret and kills existing the pod(s).
Is the thought that we would change the functionality here to always run the pipeline against the last SHA that was deployed to the scoped environment (or all environments in the case of CE currently) when you add or edit a variable?
In EE we might need to also trigger for each deployment since we can have multiple. Imagine a CI variable (if we use them) applicable to review/*. All review deployments that are still active will need to automatically trigger the pipeline if we go down this route. Do we want to do this ?
EDIT: A mitigating factor would be that we will only run the deployment part of the pipeline.
EDIT2: Perhaps we have a UI to choose which deployment to kick off.
All review deployments that are still active will need to automatically trigger the pipeline if we go down this route. Do we want to do this ?
My guess is that we would not want to trigger re deployment of old applications. Another way to mitigate this while keeping the scope small is to inform users that there changes will only take effect after the next deployment.
I think it would be nice in future if there was a way to trigger just the deployment job (and this shouldn't be too tricky with the CI feature only/except and using pipeline triggers.
It's also a little interesting that if they are stored as K8s secrets they technically don't need to redeploy the app they just need to restart the pods as @adamlem mentioned. But this is kinda hard for us to detect because if it's a new secret then we will probably need to redeploy the app since the deployment phase is where the secret is mapped to an env var.
In EE we might need to also trigger for each deployment since we can have multiple. Imagine a CI variable (if we use them) applicable to review/*. All review deployments that are still active will need to automatically trigger the pipeline if we go down this route. Do we want to do this ?
Really good point. And yet another reason to consider cutting that scope and leaving it up the user.
I think it would be nice in future if there was a way to trigger just the deployment job (and this shouldn't be too tricky with the CI feature only/except and using pipeline triggers.
For each environment, find the last pipeline that ran that deployed to that environment, re-run its deploy job (which we know because it's the one that has the environment defined).
I suspect that we can probably avoid making changes to the CI runner but I think depending on how we implement this feature there may need to be changes to the CI runner. But we can probably come up with implementations that avoid runner changes if they don't introduce too much complexity. This is really just related to the fact that we're possibly introducing a different type of CI variable and the runner won't understand this different type yet.
@DylanGriffith Right, and even if you don't introduce a new type of CI variable (because you're passing all variables through to the app), the CI script can't tell the difference between CI variables and other env variables that happened to be set, like PATH. I don't expect having the job script just take env and cat that to the Helm chart would work. You'd have to filter them somehow. One option, that kind of solves two problems at once, is to use a convention like APP_*.
the CI script can't tell the difference between CI variables and other env variables that happened to be set
Yes this is a very good point and one I forgot about while going through this discussion.
So we would need to prefix the env vars either way. Given that I don't see a strong reason not to just implement as Daniel suggested by adding a checkbox to the CI variables (or use a seperate page for this stuff). This provides much better UX than asking users to prefix their variable names. It also has the added advantage that this feature is very easily discoverable for our users.
Behind the scenes it may even be implemented by prefix named variables anyway but this is an implementation detail we'll probably decide later as there are likely many tradeoffs for maintainability.
@danielgruesso Thanks for updating with an MVCP proposal. In the interesting of aiming for truly minimal, I'm going to push even further.
Of course we don't want to pass all the variables to the app so we need a way to specify which ones to pass.
Is this really necessary? Why is this an of course? What if we just sent all variables to the app? That would presumably be easier to implement and easier to understand. If it turns out to necessary to filter some, we can always add the checkbox later. It's easier to add complexity later than it is to remove it if it's unnecessary. And we might find that it's just not necessary at all.
I'll remind you that Heroku decided to mingle compile-time and run-time variables many years ago and still haven't found the need to separate them.
If a user needs to revoke a key/value (say it got leaked) and replace it with a new one, then the pipeline would need to fully run in order for that to take effect.
@tauriedavis That depends on implementation. For runtime variables, if you push a new secret directly to k8s, you may just need to restart the pod. If you're pushing the secret via a CI/CD job, then yeah, you'd have to run the pipeline for the variable to take effect. We already have that concern with setting REPLICAS. It's currently a user action to do this, and it's fine. Automatically handling of re-running the deploy job can be a separate iteration, if needed.
For example, if a developer wants to change 5 variables, it could be really annoying to trigger the redeploy job 5 times. Letting the developer control when to make the variables take effect can be a positive.
BTW, you don't need to run an entire pipeline. We know which job did the deploy, and you can just re-run that single job.
We don't have any sort of UI today that runs just 1 job of a pipeline, right?
Would be awesome if you guys could ensure that a change in the values will update the secret and kills existing the pod(s).
That is an interesting problem. If re-running the job isn't enough, then this is definitely a problem. And you don't want the job to always kill all previous pods just in case. Detecting if variables are different before deciding to restart is a pain. That might be a good argument for treating the k8s secret store as a first-class thing, so we know definitely when we change something. It's a complexity I'd rather avoid for an MVC though.
I'd like to interject a small amount here, in regards to reloading pods at the change of secret contents.
Currently, there is no way for a pod to know that any secret content has changed. You make use of helm for the detection of the checksum of template changing (thus, through the template engine itself), but this requires using helm to install said secret (which is a best practice no-no). There is also currently no mechanism for k8s to automatically detect & "roll" pods based on alterations to the secrets that they make use of.
At this time, it will be required to manually roll the pods when secret content changes.
This can be handled by re-running the pipelines, however you may need more than just a re-run. If the template output does not differ from the previous run, Helm/Tiller will not alter the existing Deployment, and thus Pods operated by it will not be restarted. It may be required to ensure there is an item in the annotations or even an environment variable that is changed on each pipeline run, ensuring that there has been a small alteration made in order to trip the behavior you're seeking.
@WarheadsSE Good to know. Can you elaborate a bit on the Helm part? It's obviously a no-no to hard code the secrets into some Helm file, but if we're injecting them dynamically, is it still a problem having Helm manage them? And if we let Helm manage it, will it detect that a secret has changed (via checksum thingy), and thus restart pods? e.g. if the service definition hasn't changed, but the associated secret changes, does Helm know to restart? Or do we need to put the value directly in the service definition for the change to be picked up?
Passing secrets to via Helm is a no-no, in all ways. I very strongly suggest against it.
If passed as a value in any way, they can be retried with helm get values [--revision int32] RELEASE_NAME, so long as Tiller has not been purged.
If passed as a template member, they can be retrieved with helm get RELEASE_NAMEas well, in k8s YAML form, so long as Tiller has not been purged.
Yes, I am aware of the fact that if you have rights to use Helm/Tiller within a namespace, you also likely have the capability to kubectl get secrets -o yaml | less to dump all Secrets within the namespace. From a security standpoint that much is similar.
The difference then comes down to the fact of how Tiller stores the values and templates of a deployed release in $TILLER_NAMESPACE as a ConfigMap, by the name of RELEASE_NAME.v#. The impact of this, is that every deployed version, which may be a large number, would have every stored secret content ever used if passed or managed by Helm.
The impact of this, is that every deployed version, which may be a large number, would have every stored secret content ever used if passed or managed by Helm.
@WarheadsSE thanks for expanding here on the problem. This seems obvious to me now that we absolutely should not pass the values in through helm templates.
At this time, it will be required to manually roll the pods when secret content changes.
@WarheadsSE can you think of any reason this might be challenging? I'm imagining we can just have our deployment script run one command to restart all pods after the helm deploy finishes.
@WarheadsSE can you explain what you mean by no coordination?
@DylanGriffith kubernetes has special handling requirements for disruptions, but they add complexity and overhead. The case you refer to here (restarting all pods) would be referred to as a voluntary disuption. When you say "restart all the pods", this effectively means "delete all the pods and let the deployment recreate them". Thus, without something to ensure that new pods are started before the old ones are killed, you have "outage" because there are no pods servicing requests.
So it seems we probably want to follow something like Automatically Roll Deployments When ConfigMaps or Secrets change. We could maybe pass a checksum here or we could be a little hacky and just pass the timestamp which means it would always just restart pods. This would be an easier first step considering that we wouldn't need to actually know the new values in order to accomplish that. In fact depending on the implementation our CI may not even know the new values anyway if the CI environment only needs to know the env var names to map to pods.
NOTE: There was some mention of helm upgrade --recreate-pods in those docs but it seems that this also won't be suitable as it has downtime according to https://github.com/helm/helm/issues/1702
@DylanGriffith Bingo. --recreate-pods effectively performs the "kill the pods" for you. While the issue(s) linked are from a much dated past (Helm develops fast), this limitation still exists, due to the implementation.
Is this really necessary? Why is this an of course? What if we just sent all variables to the app?
@markpundsack It's not stricly necessary but it seems safer not pass variables that may or may not be sensitive to a part of the system that does not need to make use of them, but you're right, this is an assumption and could always iterate on an implementation that passes all the variables.
I'll remind you that Heroku decided to mingle compile-time and run-time variables many years ago and still haven't found the need to separate them.
This is a good point. @tauriedavis doesn't seem to me that we need to make this differentiation at all then.
Regarding the example of leaked keys, I believe for MVC is fine to have user run the relevant jobs again to re-deploy. I think the bigger issue is secret/change update behavior from a technical standpoint.
Thus, without something to ensure that new pods are started before the old ones are killed, you have "outage"
@WarheadsSE@DylanGriffith is this doable? Can we implement this "something" that ensured new pods are started before killing the old ones?
could always iterate on an implementation that passes all the variables.
@danielgruesso in this case I don't think this is a great assumption. With all features (unless we call them alpha or something) we make it very difficult to deprecate them so every future change needs to remain backward compatible. And in particular in this case we're talking about passing a bunch of variables to the app. If we decide to change that plan in future we will have broken everybody's running apps. So likely we don't want to do something "implicit" until we're sure we're happy to support this for a long time. That's why I'm much happier about an explicit approach (eg. the checkbox you added) since the user intention will be persisted in our DB and we won't be always trying to maintain compatibility without any idea about actual usage.
I'll remind you that Heroku decided to mingle compile-time and run-time variables
Yeah but Heroku is a much simpler situation here as they aren't also your CI environment. In our case we have weird env variables that may mess with your deployment (eg. KUBECONFIG being passed to your running app will cause it to have escalated privileges in your cluster). Also in GitLab's case since the variables are used for so much more than compiling and running your app then this makes less sense in GitLab. Also per my comment above I think it makes it impossible to track usage when implicit like this and as such impossible/difficult to deprecate when/if we realise this is not the right model. Also we don't even have examples of when a user would want credentials shared in production and CI and plenty of examples when they don't.
I think the bigger issue is secret/change update behavior from a technical standpoint.
This will mean we need to trigger CI to redeploy (or at least inform users to do this) but the more I think about this more I think this is inevitable anyway.
If you do decide to go the route of using checksum Annotations, then may I suggest injecting checksum/ci.environment: {{ .Values.ciEnvironmentChecksum }}, where this is a SHA2 checksum of their combined/concatenated values?
@tauriedavis I think that showing a different section for this that only appears when auto devops is enabled would be another viable option. When auto devops is enabled we could show a separate section; so we could have a section for CI Variables and another for Auto DevOps Variables (maybe even only show it when auto devops is enabled). Anyway, it may be too large for MVC but just a thought.
Also per my comment above I think it makes it impossible to track usage when implicit like this and as such impossible/difficult to deprecate when/if we realise this is not the right model.
@DylanGriffith I think you're overstating that a bit. Sure, it makes it impossible track, true. But I can imagine several easy iterations that would let us migrate away from that behavior. Adding a checkbox with a migration that defaults to true, but where new variables default to false would be one.
@danielgruesso@tauriedavis Please don't call anything Auto DevOps Variables. I'd like to think that that we're going to come up with a capability or convention that works with Auto DevOps, but that would also work elsewhere.
(eg. KUBECONFIG being passed to your running app will cause it to have escalated privileges in your cluster)
@DylanGriffith As a practical matter, the k8s configuration is part of the cluster, not part of variables, so we don't have to pass those credentials to the running app, if we don't want to. I know your comment was more generalized, but I've thought about this specific case a bit and realized we can use the cluster configuration as a way to keep that particular secret safer. Once we do that, we've actually covered quite a bit of the concerns. At least enough to seriously consider reducing scope of the first iteration; IMHO anyway.
Please don't call anything Auto DevOps Variables. I'd like to think that that we're going to come up with a capability or convention that works with Auto DevOps, but that would also work elsewhere.
@markpundsack I agree. We talked about how since we can connect to the cluster then we can provide secrets that can still be used even if not using Auto DevOps. I was thinking Cluster variables or being specific and simply calling them Compile-time variables and Run-time variables. What do you think?
Preliminary mock (I'm worried about management of lots of variables with this layout, if not in this iteration we should look into an improved layout for a next iteration):
ce
ee
Regarding combining vs. splitting the two variable types, what is the increased scope if splitting them? There is slightly more frontend work, but it's mostly duplicating what currently exists. Is the backend more difficult? Rather than scope, I think this is more about what the user expects to find, and what makes more sense within the UI, as well as what problems we may run into later. If we suspect we will need to iterate away from it in a few iterations, it may be worth starting with them separated for now if the scope is not really increased by much.
And yes, please make it independent from using Auto DevOps, I suggest to use a name that implies that it is a security feature for this particular project. How about Kubernetes Deployment Secrets
Thanks for the feedback @markpundsack, it does make sense to generalize the title more.
Thanks for the feedback @adamlem, I like the idea of conveying "security" behind the verbiage too.
@tauriedavis the layout looks good. I think the title verbiage could be related to the application or even the container; perhaps something like Container Environment Variables or Cluster Application Variables.
as well as what problems we may run into later. If we suspect we will need to iterate away from it in a few iterations, it may be worth starting with them separated for now if the scope is not really increased by much.
@tauriedavis That’s a dangerous, slippery slope. Don’t make something more complex today for what might happen later.
In this case, while the front end may not be that much more complex, the backend probably will be, including runner changes to accept the new data. But I haven’t seen a full proposal so can’t be sure.
As an alternate proposal, if not passing all variables is truly needed now, we could go with a convention of prefixing variables in a way that is detected by an Auto DevOps job so only certain variables are sent, with the prefix stripped, of course (#33527 (moved)). That could require zero backend or front end changes; just template changes. If the inconvenience of prefixing proves annoying, we could simplify with variable types to auto-prefix (#46806 (closed)). If the prefix itself proves to be a problem, we could then, and only then, update the model and runner API. But in the meantime, we will have learned more, faster, and perhaps come up with an even better solution to problems we’re not even aware of. For example, this discussion only covers runtime variables, but what about other Helm values that someone needs to set? Like memory limits? I can easily imagine adding another convention prefix for handling them, but I can’t see a clean way of extending the mock-up above to handle yet another variable type. Smaller iterations with smaller primitives usually win.
Lastly, I see the good reasons not to put secrets in Helm, but can’t help but see the conflict with using Helm the way it is intended. That’s a LOT of effort to work around Helm. Helm has all sorts of challenges, but we’ve already chosen it. We’re already putting the Postgres credentials in Helm values, for example. As pointed out, Helm 3 aims to address these things, so we can save ourselves a lot of headache by just embracing how Helm intends to work, and then benefiting from Helm 3 improvements when available. Or if we want/need to move away from Helm, so be it, but please decouple that issue.
In a future iteration on this feature, it might be helpful to look into Pod Presets (It is alpha right now, but might be useful some time next year). Examples of using Pod Preset.
But I can imagine several easy iterations that would let us migrate away from that behavior. Adding a checkbox with a migration that defaults to true, but where new variables default to false would be one.
@markpundsack I don't see how this would work. We currently have hundreds of thousands of ci variables and similar for group variables. If we start passing all of these to running apps we have no idea going forward which (if any) of these are used by the running app. Therefore we can't deprecate them. Having the checkbox enabled by default doesn't really help us with migrating data if we decide to model things differently in our database later on because we can't tell by looking at the database which table this data should be in. It's actually easier for us to merge these 2 things later if people want them to be the same than it is to split them up.
the k8s configuration is part of the cluster, not part of variables, so we don't have to pass those credentials to the running app
@markpundsack I may not have fully understood this but the way our backend code is implemented is that all of these are passed as variables to the running app. It's tricky from a backend perspective to split up all the different sources of variables and impossible for the runner to distinguish.
I'd argue we can quickly iterate if/when something more complicated becomes necessary
@markpundsack I'm not convinced the other proposals here are "more complicated". They are just appear to be more work because they involve adding a UI. Separating this stuff in our UI actually makes a lot of the code easier to write from my perspective and provides both simpler code and simpler UX. I'm not sure why adding a UI to something is making things more complicated. Explicit is generally simpler to understand and from my perspective it will be easier from the backend code perspective and only a small amount of frontend work.
Also as for iterating. This is actually my primary concern. Making decisions about implicit variables here is a particularly hard thing to change.
Please don't call anything Auto DevOps Variables
Agreed. It's probably better to come up with a more generic name here. Also Auto DevOps variables isn't helpful to the user for clarifying if the variables are available in the running app or in CI.
If we suspect we will need to iterate away from it in a few iterations, it may be worth starting with them separated for now if the scope is not really increased by much.
I'm inclined to agree. If the small "iteration" we were talking about here wasn't making it difficult to change things later then I would be more onboard but since this is a difficult to reverse decision I'm more inclined to try build out something closer to what the user expects (which has been confirmed by 2 users in this thread already).
In this case, while the front end may not be that much more complex, the backend probably will be, including runner changes to accept the new data. But I haven’t seen a full proposal so can’t be sure.
@markpundsack honestly I believe the backend will be simpler. I didn't really want to get so much into implementation details here but since this is a sticking point here is my proposal:
We store these new variables in a new model in our backend (really simple new table)
When created we create the Secret in K8s
We pass a new variable to CI jobs called APP_SECRETS which is a comma separated list of the variables (names only) that need to be mounted in the helm chart
This is not really any more complicated than the other approach because both require that we create secrets in K8s. This, if anything, is simpler to maintain because secrets are being created from our backend rather than from the CI YML script (which is very tricky to test). This is also easier to change later as the data is in a separate table so it's easy to migrate around if we decide to change things. This has the added advantage that less work is being done in the Auto DevOps template thus could be a feature used by non-auto devops apps too if documented carefully.
That could require zero backend or front end changes; just template changes
@markpundsack since this point keeps getting made I want to be very clear that template changes are backend changes. And in fact this is proving to be the most costly and difficult part of our code to maintain since it's all written in bash. If this auto devops template was not maintained and owned and supported by our backend team then I'd be onboard with this decision. In fact I wouldn't even need to do anything since the template would be built out by somebody else. But as it stands our team needs to own this and I'd much rather keep changes here to a minimum because the work is much greater than it looks.
Or if we want/need to move away from Helm, so be it, but please decouple that issue
we could go with a convention of prefixing variables
@markpundsack I see the appeal of this approach as it looks like less work to you but in reality it's just less UI. We could have tonnes of features in GitLab controlled by specially named env vars but it doesn't lead to a great UX. And this is @tauriedavis's domain and I think she's said several times that this is not how she wants us to tackle this problem. I'm very inclined to agree here that it provides poor UX and doesn't really save us from much work.
That’s a dangerous, slippery slope. Don’t make something more complex today for what might happen later.
This wasn't really what I was getting at. I didn't mean to imply that we should make it more complex, just that it may not actually be more complex which @DylanGriffith seems to confirm above.
Is the current proposal to always create Kubernetes secrets which the CI pipeline can leverage ? Regardless of where the CI is running Auto DevOps or not ?
If so, then I would suggest Application secrets with a footnote it's only available with Kubernetes :)
Well since we're no longer going with a dedicated section this is no longer applicable. Still worth discussing proposed variable name in different thread.
@tauriedavis another thought on UX, how do you feel about a combo box where user can select variable type instead of a whole new section? ie:
Of course we would have to write rules such as environment being available if variable type is X, or "protected" not being actionable is variable type is Y, etc.
Talked with @markpundsack again last Friday and we agreed that it would be good for both @markpundsack and @DylanGriffith (+ others) to articulate the proposals and the pros/cons of each proposal. I know there is already a lot in the comments above but lets focus on clearly articulating each point so we can arrive at an informed decision.
I am fine to tackle this however we like from a UX perspective and I would like Taurie and Daniel to own that decision. My preference would be to avoid implicitly passing every variable to the running application as this is a hard to reverse decision. Secondly I believe the backend code will be simpler to maintain (and just as quick to write) if we explicitly model these variable types somehow (checkbox or other) rather than try to pull apart implicit variable naming conventions like K8_.
I'm not sure if there is more detail you want here but I think these are the only 2 that relate to UX. All the other discussions amount to implementation details which I'm not sure impact any immediate decision.
One concern I have about pushing variables to k8s directly from GitLab Rails at variable creation/update time is keeping them in sync with clusters. i.e. what happens if I add a new cluster for production, especially at the group level? Will we have to loop over all the project/group variables for all group projects to apply them to the new cluster? If the variables are sent as part of CI/CD, they'd naturally be sent when the app is deployed. I'm sure there are ways to solve it, such as always re-applying k8s variables before every pipeline run, but that adds some complexity that should be accounted for.
In the last call, I mentioned one potential advantage of using variable name prefixing (like k8s_secret_) was that it would work at the group and project levels immediately, and also at the runner level if someone wants to define variables in the runner that should be passed to the application. Another usage that came up after is variables declared in .gitlab-ci.yml. Not all variables need to be "secret", and using the prefix convention allows people to declare values in YML that would be passed to the app.
@markpundsack I think this message belongs here where we are discussing UX.
Just a thought I had is that even if the implementation from a CI perspective ends up being prefixed variables (considering it may be difficult to change the runner another way) this does not limit us to using variable name prefixing from a UX perspective. I wanted to decouple these conversations since I think it is easy enough for us to have a UI where a user has a checkbox but behind the scenes this is converted to a prefix variable name before being passed into the runner OR we pass it the the runner as a completely seperate env var with all secrets. We have many possibilities.
The point being that I really wanted to focus on the UX here. Do we want users to be exposed to a variable naming convention or would we rather additional checkbox or something else? I don't think either of these effect the implementation really so that's why I wanted Taurie and Daniel to own the decision. And from my perspective I'm happy with however you decide (prefix, checkbox, drop down) considering any of them can be hacked together to the same backend implementation.
Just a thought I had is that even if the implementation from a CI perspective ends up being prefixed variables (considering it may be difficult to change the runner another way) this does not limit us to using variable name prefixing from a UX perspective.
Correct, but also, we can decouple implementation that way, and have one iteration where the user must prefix, and a subsequent iteration where we do that for them. Even if we can do both in one release, it's best to be able to decouple and ship independently.
Correct, but also, we can decouple implementation that way, and have one iteration where the user must prefix, and a subsequent iteration where we do that for them
Sure. I'm fine with whichever of those 2 approaches we take so I'll leave it up to Taurie and Daniel. One approach involves more docs and the other approach involves a little more code in frontend and to stick a prefix onto the variable before sending to the runner. I'm also fine with this prefixing approach since I think it should be straightforward to migrate data in future if we decide to store it in the DB without the prefixes later.
@markpundsack based on the conversations here it seems that either prefixing or having a separate section will be the same amount of work from a backend perspective. Even if adding this later on for group-level is copy/paste, the same could be said for doing it in a separate section.
The reason I prefer to do it in a separate section comes down to:
We don't have to deal with extra logic in CE to allow user to scope it to an environment
We don't confuse the user by showing the "protected" label when in reality this is doing nothing for k8s secrets
We don't make the user learn a new convention for prefixing variables
The front-end effort to implement this is minimal. Bringing it to group-level is still copy/paste.
So since we've established that the variables will be sent as part of CI/CD so they are sent when the app is deployed, they should be able to leverage the existing logic variables have. "protected" among those. The use case here would be as for all other protected variables, where they are only passed to a particular protected branch. We could offer "protected environments" to solve for this nonetheless but it's a good point towards consistency.
@tauriedavis since we will be passing as part of CI/CD, the prefixing approach should work out of the box for group-level variables as well. That in addition to the fact that we may want to pass other variable types in the future is a good reason to use the prefixing approach instead.
Yes, I think we are all in agreement about how the prefixing approach works for group level variables and possibly other variable types in the future.
The question left is how we deal with protected variables and being able to scope an environment in CE. Sounds like we can just keep protected variables assuming the implementation accounts for that. That leaves environment scope.
From my perspective, the quickest way to account for this is to either:
A) Add environment to CE. Sounded like people didn't feel this was very MVC-material though.
B) Add a separate section so that users can always add an environment to these variables. We could of course add some more complicated UI that allows users to choose the variable type but that is not the boring solution for MVC.
I feel like we keep flipflopping without addressing all the points raised above. Regardless of whether we prefix or not, lets solve for the environment problem.
From my pov, we don't have to deal with protected variables at all. One of the benefits. Scoping to an environment in CE simply won't be available for MVC which is ok. I think the thing to do here is port the ability to scope variables to environments to core. We will decouple that though.
I created a research issue to investigate some of the assumptions we are making here. Feel free to contribute your own questions/assumptions to the issue @DylanGriffith@markpundsack@danielgruesso
In the last call, I mentioned one potential advantage of using variable name prefixing (like k8s_secret_) was that it would work at the group and project levels immediately, and also at the runner level if someone wants to define variables in the runner that should be passed to the application. Another usage that came up after is variables declared in .gitlab-ci.yml. Not all variables need to be "secret", and using the prefix convention allows people to declare values in YML that would be passed to the app.
@markpundsack let's move the implementation detail discussion to this thread if that's ok.
One concern I have about pushing variables to k8s directly from GitLab Rails at variable creation/update time is keeping them in sync with clusters. i.e. what happens if I add a new cluster for production, especially at the group level? Will we have to loop over all the project/group variables for all group projects to apply them to the new cluster? If the variables are sent as part of CI/CD, they'd naturally be sent when the app is deployed. I'm sure there are ways to solve it, such as always re-applying k8s variables before every pipeline run, but that adds some complexity that should be accounted for.
This is a really good point and one I had not considered. This is a good enough reason for me to think we should create them in CI rather than in our backend initially considering the difficulty of multiple clusters. I really wish we could try to resolve that longer term and come up with something coherent that was doing things in a more Kubernetes integrated way and not relying so heavily on our CI scripts but I think I'm fine with implementing it this way for now.
I think it should be easy enough to avoid the security issues of creating in Helm too by simply just adding steps to the deploy script that create a Secret before we do the helm deploy. I don't think this makes it any more difficult since we either add the looping logic to the helm chart or we add it the the bash script but at least this way we mitigate helm's security problems.
I think it should be easy enough to avoid the security issues of creating in Helm too by simply just adding steps to the deploy script that create a Secret before we do the helm deploy.
Agreed. If that's important, it's easy to mitigate. Note that there are further edge cases here to consider, such as variable deletions. We'd have to detect when a variable is no longer declared and delete it from the k8s secrets, whereas if we used Helm to manage the secrets, it would handle that automatically. Might still be worth it, of course.
I really wish we could try to resolve that longer term and come up with something coherent that was doing things in a more Kubernetes integrated way and not relying so heavily on our CI scripts but I think I'm fine with implementing it this way for now.
Note that there are further edge cases here to consider, such as variable deletions. We'd have to detect when a variable is no longer declared and delete it from the k8s secrets
Since K8s Secret can store multiple things in one resource (as documented https://kubernetes.io/docs/concepts/configuration/secret/#creating-a-secret-manually ) it should be possible to just always replace the entire secret and store all env vars in one secret. Not sure if this will be easier than the helm approach of creating one secret per env var but if we didn't use helm this would certainly solve the problem about deleted variables.
Secrets & configMaps can indeed hold many more than one entry. The item to watch for is running into a limitation to the size of a map, but that's pretty hard to hit. These can then be sent directly as env to a pod/container with the form of secretKeyRef per documentation
It seems you have decided on prefixing and letting the Gitlab Runner script handle the variables.
You also argue that it will be simple to migrate away from this approach (I assume, by continuing to supply the variables prefixed to the Gitlab Runner, to not break scripts expecting them).
From security standpoint I can imagine some situations where it is undesirable to let a certain Gitlab Runner know the secrets, but I might still want to run some job through that runner on a protected branch. It might be some 3rd party static security testing job.
Letting the Gitlab Runner script take care of setting the Kubernetes secrets now, will make a migration somewhat more difficult if you later decide to not give the secrets to all the runners.
In a future iteration on this feature, it might be helpful to look into Pod Presets (It is alpha right now, but might be useful some time next year).
@rp1 I thought this Pod Presets option looked great and possibly close to being the simplest approach. You say that it's alpha right now. I wonder how risky it would be for us to tie ourselves to this implementation in practice.
From security standpoint I can imagine some situations where it is undesirable to let a certain Gitlab Runner know the secrets, but I might still want to run some job through that runner on a protected branch. It might be some 3rd party static security testing job.
@rp1 That is a good point, but something we think we can iterate to later, if necessary. I also caution against the illusion of security here. If the runner has secrets to deploy the app, a malicious job on the runner could always use kubectl to fetch all the secrets anyway. So hiding them from the runner only makes it slightly more difficult, not actual safe.
I updated the description to remove the mocks and include an update to the descriptive text within the UI @danielgruesso. I think the rest is backend changes but I'll leave myself assigned incase anything comes up!
In a future iteration on this feature, it might be helpful to look into Pod Presets (It is alpha right now, but might be useful some time next year).
@rp1 I thought this Pod Presets option looked great and possibly close to being the simplest approach. You say that it's alpha right now. I wonder how risky it would be for us to tie ourselves to this implementation in practice.
Please be very wary of alpha features, as you also lock users into newer versions than they may have available to them on-prem or from their authorized vendors.
Not that I'm suggesting we change anything right now but I wanted to at least document here a significant security tradeoff we're making in our design of this feature. It's not clear what a good alternative is so we may need to just keep moving forward but we should probably document this really clearly as to why it matters to our users.
Any K8s process with DIND access can read the environment of any container running on the same K8s node
When installing GitLab runner from GitLab we install using DIND (needed for docker build)
GitLab Runner (by design) allows any developer with access to any repo that is using this runner to run any arbitrary code at all including the commands necessary to extract secrets from other apps deployed to the cluster
This feature is encouraging users to configure production secrets as env vars
So basically if users follow all our instructions using our features we should expect that any developer in their organisation will be able to read any production secret configured for any GitLab deployed applications in their cluster.
I propose a couple of follow-ups:
This is documented clearly with this new Auto DevOps secrets feature
Proposal to workaround this is to tell users they can install their runner in a different cluster OR not in a cluster at all OR encourage them to use our shared runners on GitLab.com unless they can't for some reason
Also consider a follow up issue to not use DIND by default for the runner (meaning we need to change all docker commands in Auto DevOps to use kaniko or something that doesn't require DIND)
Interesting ! What else can a privileged process do ?
This is documented clearly with this new Auto DevOps secrets feature
Proposal to workaround this is to tell users they can install their runner in a different cluster OR not in a cluster at all OR encourage them to use our shared runners on GitLab.com unless they can't for some reason
Will do
Also consider a follow up issue to not use DIND by default for the runner (meaning we need to change all docker commands in Auto DevOps to use kaniko or something that doesn't require DIND)
GitLab Runner (by design) allows any developer with access to any repo that is using this runner to run any arbitrary code at all including the commands necessary to extract secrets from other apps deployed to the cluster
This feature is encouraging users to configure production secrets as env vars
I read https://gitlab.com/charts/gitlab/issues/114#note_113106981 and think that privileged is the issue. It seems tangential that that this feature uses Environment variables, as any actor with access to the privileged container and perform any docker exec action.
Put another way, mitigating point 4 (env var) is about reducing attack surface as, noted in point 3, using another way still has the secret being present in the pod which is still accessible via arbitrary code.
ps. Actually isn't this wider than Auto DevOps ? GitLab runner is GitLab managed apps is installed with privileged. So any dev can run a branch with dind, for some example.
I read https://gitlab.com/charts/gitlab/issues/114#note_113106981 and think that privileged is the issue. It seems tangential that that this feature uses Environment variables, as any actor with access to the privileged container and perform any docker exec action.
Spot on. privileged is the problem, even if not using GitLab Runner.
Put another way, mitigating point 4 (env var) is about reducing attack surface as, noted in point 3, using another way still has the secret being present in the pod which is still accessible via arbitrary code.
Spot on again, as I've pointed out in the mentioned slack thread, and in my response to charts/gitlab#114's comments. They way we're doing things within the cloud native GitLab (CNG) helm charts is done to reduce immediate attack surface, as it is impossible to completely prevent a truly determined presence from getting to key information with enough knowledge of the active systems. We're not "just" obscuring the information, but requiring a higher amount of work for the return. Without a known method to fully prevent the concerns, we simply have to do our best to not be an easy target. With a carefully constructed system and permissions system in an active production environment, these types of things can be mitigated through administration, but most of the CNG chart users won't be that careful.
And this is the precise concern that many users will not be aware of. The problem is never documentation, its ensuring understanding. We can, and do, document this well, and in many places. We can only do our best to educate the users.
And this is the precise concern that many users will not be aware of. The problem is never documentation, its ensuring understanding. We can, and do, document this well, and in many places. We can only do our best to educate the users.
Thanks @WarheadsSE - we will have work to continue the education here and to improve GitLab
Also consider a follow up issue to not use DIND by default for the runner (meaning we need to change all docker commands in Auto DevOps to use kaniko or something that doesn't require DIND)
There is an edge case in Secrets we should discuss. User updates Secret variables but does not do code change, then runs Auto DevOps. This results in... Pods not being recreated, and they will have old secret variables.
Mitigations user can do:
Force some code change
Manually delete pods. Kubernetes will then create pods which will have updated secret variables
I will not try to automate forcing through a secrets update with no code change as I feel this should belong in a follow up issue. WDYT ?
If you do decide to go the route of using checksum Annotations, then may I suggest injecting checksum/ci.environment: {{ .Values.ciEnvironmentChecksum }}, where this is a SHA2 checksum of their combined/concatenated values?
I am fine with a follow up MR and even issue if that helps but I don't believe that this is an edge case. I believe that this is something users will run into very quickly so we should follow up pretty much straight away otherwise users are likely to get fairly confused about why their apps are still complaining that their new secret is not available. People are unlikely to combine code changes with env changes so probably we'll see lots of users run into this when trying to use the feature.
@danielgruesso to weigh in if he doesn't think this is a priority but I think we should do this next before we move on from this feature because it's pretty straightforward and it is very valuable.
Daniel Gruessochanged title from Configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Configure Auto DevOps app secrets outisde the repo
changed title from Configure Auto DevOps deployed applications with secrets that aren't committed to the repo to Configure Auto DevOps app secrets outisde the repo
Daniel Gruessochanged title from Configure Auto DevOps app secrets outisde the repo to Configure Auto DevOps app secrets outside the repo
changed title from Configure Auto DevOps app secrets outisde the repo to Configure Auto DevOps app secrets outside the repo
Mark explained to me why we needed a prefix and I wanted to share it in case other people wondered: It didn’t require any chances to what we send to the runner, just a convention and changes to Auto DevOps template to understand the convention. Customers don’t want to send all secrets as some credentials should only be available to CI/CD. E.g. the creds to k8s itself shouldn’t be passed to the app. That increases surface area of security attacks. There are multiple types of variables. Kubernetes has “config” and ‘secret” for example, but there’s also Helm variables. This convention extends well to supporting different types, while also happening to be the simplest solution, so win/win. Sending all secrets is actually not easier as we’d have to add a mechanism to let the runner know which variables are GitLab secrets vs just env variables the runner happens to have. E.g. you can’t send the job’s PATH to your app and expect it to still work.
1
Daniel Gruessochanged title from Configure Auto DevOps app secrets outside the repo to Configure Kubernetes app secrets as variables
changed title from Configure Auto DevOps app secrets outside the repo to Configure Kubernetes app secrets as variables
Daniel Gruessochanged title from Configure Kubernetes app secrets as variables to Configure Kubernetes app secrets as variables for Auto DevOps Apps
changed title from Configure Kubernetes app secrets as variables to Configure Kubernetes app secrets as variables for Auto DevOps Apps
I'm using Gitlab CI Auto Devops (Gitlab.org) with K8S cluster integration and Gitlab specific runner. I have a simple dockerize Node.js app which I would like to deploy and pass secret from Gitlab CI Variables to Node.js varenv (eg. PORT and HELLO). I found this documentation from Gitlab: https://docs.gitlab.com/ee/topics/autodevops/#application-secret-variables
I added K8S_SECRET_HELLO = WORLD as Gitlab CI variable (it can be a secret for example)
Dockerfile
FROM node:12-alpineWORKDIR /appCOPY package*.json ./RUN npm ci --productionCOPY . .ENV PORT 5000RUN echo"HELLO: $HELLO"# => I have 'undefined'CMD [ "node", "." ]
index.js
constexpress=require('express');console.log('Hello: '+process.env.HELLO);// => I should have "WORLD" but I have "UNDEFINED"constPORT=process.env.PORT;// => I have "5000" but I can't override it from Gitlab CI variableconstapp=express();app.get('/',(req,res)=>res.json({env:process.env}));// => I check all varenv hereapp.listen(PORT,()=>console.log('Running on http://localhost:'+PORT));
After I ran the pipeline (build and production jobs passed successfully), HELLO varenv is not set and I can't access from my app. However, the documention said about K8S_SECRET:
any variable prefixed with K8S_SECRET_ will be made available by Auto DevOps as environment variables to the deployed application.
Did I miss something ?
Maybe someone has the Answer (= 42) :) Thanks for any help!