Skip to content

engine.#NewSecret support#1307

Merged
aluzzardi merged 3 commits into
dagger:mainfrom
aluzzardi:engine-loadsecret
Jan 10, 2022
Merged

engine.#NewSecret support#1307
aluzzardi merged 3 commits into
dagger:mainfrom
aluzzardi:engine-loadsecret

Conversation

@aluzzardi
Copy link
Copy Markdown
Contributor

@aluzzardi aluzzardi commented Dec 23, 2021

Fixes #1305

/cc @grouville @TomChv

I know you run into problems with secrets being either string and #Secret -- this should fix that problem.

@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 23, 2021

✔️ Deploy Preview for devel-docs-dagger-io ready!

🔨 Explore the source changes: 3e1ce901fa0aad8dd5996d36f66e4738a766585c

🔍 Inspect the deploy log: https://app.netlify.com/sites/devel-docs-dagger-io/deploys/61d89b413704fe000807044a

😎 Browse the preview: https://deploy-preview-1307--devel-docs-dagger-io.netlify.app

@aluzzardi aluzzardi requested a review from shykes December 24, 2021 01:14
@talentedmrjones
Copy link
Copy Markdown

Seems like this could be useful for things like aws-vault where the temporary creds are never exposed to the client but only within buildkit. @jlongtine thoughts?

@jlongtine
Copy link
Copy Markdown
Contributor

@talentedmrjones Yeah, I like that idea. I think I need a bit more understanding of how it would work. I have some intuitive concerns about security, but I'm not sure I can articulate them well yet.

@TomChv
Copy link
Copy Markdown
Member

TomChv commented Jan 6, 2022

@aluzzardi That should solve our problem! We will need to update docker package to only supports secrets.

@aluzzardi aluzzardi changed the title [FOR DISCUSSION] engine.#LoadSecret support engine.#NewSecret support Jan 7, 2022
@aluzzardi
Copy link
Copy Markdown
Contributor Author

Addressing @shykes comment #1305 (comment)

I have nits on the DX (of course):

contents: this is not the actually the contents of the secret, so the field name is confusing. output makes more sense ot me, but open to other suggestions.

Changed to output

The definition name #LoadSecret. I feel like we can find something more clear, but not 100% sure what. I think maybe #NewSecret (“create a new secret backed by the contents of a file”)

Changed to #NewSecret

It seems inevitable that developers will ask: “please also support executing a command just like input.secret, and so on until feature parity with inputs. How should we respond when they do?

I don't have an answer to that. We technically can support that, but the entire point of inputs is not to.

@shykes
Copy link
Copy Markdown
Contributor

shykes commented Jan 7, 2022

It seems inevitable that developers will ask: “please also support executing a command just like input.secret, and so on until feature parity with inputs. How should we respond when they do?

I don't have an answer to that. We technically can support that, but the entire point of inputs is not to.

To clarify, I didn’t mean “please let us execute a command on the client”. I meant “please let us execute a command inside a container

@aluzzardi
Copy link
Copy Markdown
Contributor Author

It's doable with #Exec + #NewSecret

Fixes #1305

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
@aluzzardi
Copy link
Copy Markdown
Contributor Author

Ping

Copy link
Copy Markdown
Contributor

@shykes shykes left a comment

Choose a reason for hiding this comment

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

Looks good. If you could just add a comment on the parent issue with the new spec, for easier searching down the road.

@aluzzardi
Copy link
Copy Markdown
Contributor Author

Done

@aluzzardi aluzzardi merged commit c512641 into dagger:main Jan 10, 2022
@aluzzardi aluzzardi deleted the engine-loadsecret branch January 10, 2022 20: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.

Support loading secrets from actions

5 participants