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

Support for file projection in run command #141

Merged
merged 2 commits into from
Oct 27, 2023
Merged

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Oct 19, 2023

Overview

This PR adds support for projecting configuration properties as temporary files, based on a new well-known block called files. For each property, a temporary file is created containing the property value, and an environment variable is set pointing to the temporary file. Note that the file contents may be dynamically generated using interpolation.

Example

This feature is designed for a key scenario of projecting a kubeconfig file based on a Pulumi environment. Here's how that would work:

Define an environment:

# import an environment that provides AWS credentials (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`).
imports:
  - dev-sandbox

# define a kubeconfig file.
values:
  files:
    KUBECONFIG: |
      apiVersion: v1
      clusters:
      - cluster:
          certificate-authority-data: LS0tLS1...
          server: https://....gr7.us-west-2.eks.amazonaws.com
        name: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
      contexts:
      - context:
          cluster: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
          user: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
        name: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
      current-context: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
      kind: Config
      preferences: {}
      users:
      - name: arn:aws:eks:us-west-2:616138583583:cluster/mycluster
        user:
          exec:
            apiVersion: client.authentication.k8s.io/v1beta1
            args:
            - --region
            - us-west-2
            - eks
            - get-token
            - --cluster-name
            - mycluster
            - --output
            - json
            command: aws

Run the environment and observe there's a KUBECONFIG environment variable that points to a temporary file containing the desired configuration:

$ esc run -i eron-kubernetes-test -- sh -c 'cat $KUBECONFIG'
apiVersion: v1
clusters:
- cluster:
...

This enables:

$ esc run -i eron-kubernetes-test -- kubectl get ns
NAME              STATUS   AGE
default           Active   46d
kube-node-lease   Active   46d
kube-public       Active   46d
kube-system       Active   46d

@EronWright EronWright requested a review from pgavlin October 19, 2023 22:33
@lukehoban
Copy link
Contributor

What are the semantics of files for consumption by tools other than esc run where there is not an "end time" for cleanup? Or is this block something that only matters for esc run?

I feel like perhaps we want to approach this in a way that doesn't require a new top level special key? Is there any option for that? Where we could somehow have the consumer (esc run in this case) be explicit about the keys it wants to map? Like esc run myenv --file kubeconfig:KUBECONFIG -- kubectl get ns?

@EronWright
Copy link
Contributor Author

The lack of an "end time" is why I focused on run. I'm new to the project but I get the sense that various commands operate over well-known subsets. For example, esc open -f dotenv works exclusively with environmentVariables. Special keys seem foundational in Pulumi ESC and not to be feared.

@pgavlin
Copy link
Member

pgavlin commented Oct 24, 2023

Special keys seem foundational in Pulumi ESC and not to be feared.

I agree with this, FWIW.

I'll take a look at this PR today or tomorrow. I have a feeling that I'm going to need to give it some bake time.

One aside--I suspect is that it will be a best practice to define things like the contents of the kubeconfig in a "normal" value, then reference that from projections. Maybe I'm wrong about that but it smells promising to me, as the definition can then be shared around various projections as necessary. In this case that might mean defining the kubeconfig itself directly under values, then projecting it as a file by referencing it like

  files:
    KUBECONFIG: ${kubeconfig}

Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall idea looks great--just one comment on also writing these files in open when --format is shell or dotenv.

Comment on lines +182 to +197
if files, ok := env.Properties["files"].Value.(map[string]esc.Value); ok {
for k, v := range files {
if strValue, ok := v.Value.(string); ok {
file, err := writeTempFile(strValue)
if err != nil {
return err
}
defer func() {
rmErr := envcmd.esc.fs.Remove(file)
contract.IgnoreError(rmErr)
}()

environ = append(environ, fmt.Sprintf("%v=%v", k, file))
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should also write these files in open for --format=shell and --format=dotenv.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(LMK if you want to have a go at that. if not we can take this as-is and add support for open in a subsequent PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that we tackle it in a follow-up, to keep this PR as focused as possible.

@lblackstone
Copy link
Member

Do we want to stick with files as the section name, or should we be explicit about the fact that they're ephemeral? Something like ephemeralFiles, perhaps? We discussed that there may be other use cases for files that persist.

@pgavlin
Copy link
Member

pgavlin commented Oct 24, 2023

Do we want to stick with files as the section name, or should we be explicit about the fact that they're ephemeral? Something like ephemeralFiles, perhaps? We discussed that there may be other use cases for files that persist.

Personally I'm good with files. If we were to change the name I'd suggest tempFiles.

@EronWright EronWright merged commit fec257c into main Oct 27, 2023
@EronWright EronWright deleted the eronwright/files branch October 27, 2023 16:09
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

Successfully merging this pull request may close these issues.

4 participants