Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declarative secrets #198

Closed
bdmac opened this issue Feb 7, 2023 · 15 comments
Closed

Declarative secrets #198

bdmac opened this issue Feb 7, 2023 · 15 comments
Milestone

Comments

@bdmac
Copy link
Contributor

bdmac commented Feb 7, 2023

Proposal

Allow secrets to be declared in devcontainer outlining requirements to successfully run the container. All declared secrets in this initial specification would be injected into the container as ENV variables.

Declared secrets would be recommendations not requirements. Missing secrets should not prevent creation of the container.

Example

{ 
  "secrets": [
    { "name": "MY_SECRET", "description": "This is a great description of MY_SECRET." }
  ]
}

Related to https://github.com/github/codespaces/issues/12040

@joshspicer
Copy link
Member

Would an optional boolean required for each secret make sense to include (defaults to false) ?

@lostintangent
Copy link

lostintangent commented Feb 7, 2023

If a secret represents an external API key, then it might be helpful to allow the repo maintainer to include the URL to retrieve it. That way, the experience feels as actionable as possible (e.g. "You need this secret and here's where to get it!"). I could imagine that being enabled by allowing the description to be Markdown (and we render links in the UI) and/or adding an optional url property to the secret schema.

@bdmac
Copy link
Contributor Author

bdmac commented Feb 7, 2023

I had planned for our descriptions to just allow HTML snippets and run them through some sanitization in Rails-land. Markdown would also be fine. See the (terrible MVP-only) UI mock here: https://github.com/github/codespaces/issues/12175

@chrmarti
Copy link
Contributor

I assume this is for the devcontainer.json? Would the devcontainer-feature.json also support the declaration of secrets? Would they go into the image metadata?

Not every UI might be able to or allow for rendering Markdown / HTML (e.g., VS Code's QuickPick UI). We could keep the description plain-text and add an optional "documentationURL" property to allow for more flexibility in the UI.

I suggest we use a JSON object mapping secret names to their properties, that will align well with existing properties like "containerEnv" and "remoteEnv":

    "secrets": {
        "MY_SECRET": {
            "description": "This is a great description of MY_SECRET."
        }
    },

Instead of making secrets implicitly part of the environment variables, we could use a similar syntax to the one we are using, e.g., to include local variables ${localEnv:SOME_VARIABLE} in the devcontainer.json and reference secrets as ${secret:MY_SECRET}. That would allow for additional flexibility in where and how secrets can be used:

{
    "image": "mcr.microsoft.com/devcontainers/base:bullseye",
    "secrets": {
        "MY_SECRET": {
            "description": "This is a great description of MY_SECRET."
        }
    },
    "remoteEnv": {
        "MY_SECRET": "${secret:MY_SECRET}",
        "SOME_VARIABLE": "${localEnv:SOME_VARIABLE}"
    },
    "features": {
        "ghcr.io/devcontainers/features/my_feature:1": {
            "version": "latest",
            "accessToken": "${secret:MY_SECRET}"
        }
    }
}

@Chuxel
Copy link
Member

Chuxel commented Feb 13, 2023

Instead of making secrets implicitly part of the environment variables,

@chrmarti I think by precedent, the secrets should be injected as environment variables by default. We could have a type on the secret over time. Given the amount of existing use, I think we need to mirror Codespaces in that regard. Too much existing precedent there to vary IMHO.

That said, everything else makes sense to me.

@Chuxel
Copy link
Member

Chuxel commented Feb 15, 2023

Ok, so here's the current proposal summary after a few other conversations:

  1. To maintain compatibility, we can have secrets that are declared end up as environment variables by default. It is up to the implementing tool / service to ensure that secrets are not added too permissively. (e.g., user secrets have to be explicitly assigned to a repository in Codespaces for them to appear) This makes sense, since only the invoking system can know this context, and devcontainer.json itself is inherently shared in general - forcing a declaration really doesn't provide added security as a result. For example, the VS Code team needs to consider how secrets should be set for the Dev Containers extension in general. (A .env file in a specific location? Something else? How should access to secrets be restricted?)
  2. However, to allow users to opt out of environment variables being set, we can add a flag into the secret declaration (e.g. "autoInject": false or "type": "reference").
  3. In either case, a ${secret:<blah>} syntax can be used to reference these elsewhere in the devcontainer.json. Right now this does not provide guarantees that this would not result in the secret being placed somewhere in the container, but it provides a useful way to inject user level secrets and parameters into Features in general.
  4. However, secret requirements can also appear in devcontainer-features.json since Features can have explicit requirements. Once again, the point of control on what goes in them is within the implementing system's control.
{
    "image": "mcr.microsoft.com/devcontainers/base:bullseye",
    "secrets": {
        "MY_SECRET": {
            "description": "I will show up as an env var.",
            "type": "environmentVariable"  // The default
        },
        "MY_NON_ENV_SECRET": {
            "description": "I will not show up as an env var.",
            "type": "reference"
        }
    },
    "features": {
        "ghcr.io/devcontainers/features/my_feature:1": {
            "version": "latest",
            "accessToken": "${secret:MY_NON_ENV_SECRET}"
        }
    }
}

Seem workable?

@bdmac
Copy link
Contributor Author

bdmac commented Feb 15, 2023

@Chuxel I don't think most of that impacts us directly on Codespaces so that seems fine to me. Just to clarify a minimal implementation for Codespaces would now look something like this correct?

{
    "image": "mcr.microsoft.com/devcontainers/base:bullseye",
    "secrets": {
        "CHAT_GPT_API_KEY": {
            "description": "I'm your super cool API key for ChatGPT.",
            "documentationUrl": "https://openai.com/api/"
        }
    }
}

@Chuxel
Copy link
Member

Chuxel commented Feb 15, 2023

@Chuxel I don't think most of that impacts us directly on Codespaces so that seems fine to me. Just to clarify a minimal implementation for Codespaces would now look something like this correct?

{
    "image": "mcr.microsoft.com/devcontainers/base:bullseye",
    "secrets": {
        "CHAT_GPT_API_KEY": {
            "description": "I'm your super cool API key for ChatGPT.",
            "documentationUrl": "https://openai.com/api/"
        }
    }
}

Yeah this should get added to the devcontainers CLI as a reference implementation, which would allow all those things to work. But an implementation outside of the CLI could start with just that, yes. There's a spot for tool/service specific features and and limitations we can add it to in containers.dev.

@bdmac
Copy link
Contributor Author

bdmac commented Mar 21, 2023

@bamurtaugh with the proposal merged do we close this issue now?

@bamurtaugh
Copy link
Member

@bdmac Yes! And then you open a new issue labeled finalization: finalization Proposal to be made part of the spec , so that anyone from the community can provide thoughts to help continue shaping it.

I'd also recommend sharing the finalization issue in the community Slack channel for greater awareness.

@bdmac
Copy link
Contributor Author

bdmac commented Mar 22, 2023

@bamurtaugh I opened #216 for finalization but my access won't let me add labels.

@bamurtaugh
Copy link
Member

Thanks! I'll add it for you.

@afcarvalho1991
Copy link

afcarvalho1991 commented Apr 25, 2023

Not entirely sure if this also applies but we intend to use Doppler (link) which is responsible for managing our environment variables.

Right now, the problem is that we have to pass a Token as an environment variable which allows the container to obtain the environment variables needed for the container to operate.
I was not able to find any information about this but the idea would be.

  1. automatically generate a temporary Doppler token (valid for X minutes/hours) and store it as an environmental variable (@host) and pass it into the devcontainer.
  2. Always have the token being generated using something like export DOPPLER_TOKEN=$(doppler configs tokens create token-name --plain --max-age 120m) before starting the devcontainer build/start.

If the above does not happen, then we have to explicitly pass the token to the devcontainer.json and manually generate a new token every now and then, risking having the token submitted to our git repo which is not great.

Questions:

  • Is there a way to run a command before a build or starting/run a container?
  • Passing an envFile could work if we were able to run doppler configs tokens create token-name --plain) > .env.temp before launching the container, as these tokens expire after X amount of time it would be reasonable.
  • Can I start or build a devcontainer via the terminal instead of using the VScode command palette?

Thanks!
And great work for having these tools being developed which addresses many dev containerization challenges.

@marcindulak
Copy link

For 2. initializeCommand can be used. There were some issues with this approach, but I believe the functionality is expected to stay microsoft/vscode-remote-release#7377

@afcarvalho1991
Copy link

For 2. initializeCommand can be used. There were some issues with this approach, but I believe the functionality is expected to stay microsoft/vscode-remote-release#7377

I think this might do the trick... I'll try it tomorrow! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants