feat: extends mounted secret api #5707
Conversation
|
Only netlify failed, not related to the PR, I add the release note and we're good to merge |
|
@TomChv, don't create changie logs in SDKs if it's just the auto-generated code. This should just be an engine change. Each SDK release points to the engine release it's compatible with, and the change will be listed there. |
TomChv
force-pushed
the
feat/extends-mounted-secret-api
branch
from
d5e763e
to
187d879
Compare
|
@wingyplus PR updated, I switched back to camelcase |
I regenerate all clients and we shall be good to merge |
|
LGTM. 🚀 |
core/schema/container.graphqls
Outdated
| """ | ||
| Permission given to the mounted secret (e.g., 0600). | ||
|
|
||
| Default: 0644. |
There was a problem hiding this comment.
| Default: 0644. | |
| Default: 0400. |
core/integration/container_test.go
Outdated
| Optional: true, | ||
| }) | ||
|
|
||
| _, err := ctr.WithExec([]string{"sh", "-c", "test $(cat /secret) = 'secret'"}).Sync(ctx) |
There was a problem hiding this comment.
As far as I can tell, this test doesn't actually check whether the secret was optional, or that the configured mode was respected. I think we need a test for each. You can run stat to see the mode, and test -z "$(cat /secret)" to test that it's empty.
There was a problem hiding this comment.
I fixed the test and doc, but I'm not sure about optional? How can I mount an actual optional secret? @vito
There was a problem hiding this comment.
Yeah it seems like you can't, so I wouldn't bother including an API for it. In Buildkit this is a thing because secret names are configured in a Dockerfile and optionally provided by the user separately, but Dagger handles both together; an optional secret would probably just amount to passing an empty string to SetSecret.
There was a problem hiding this comment.
Then should I remove the option?
There was a problem hiding this comment.
Yeah, if we can't come up with a test for it, to me that proves that we don't need it.
Resolves: dagger#3323 Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Co-authored-by: Thanabodee Charoenpiriyakij <wingyminus@gmail.com> Signed-off-by: Vasek - Tom C <tom@quartz.technology>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
Signed-off-by: Vasek - Tom C <tom@epitech.eu>
TomChv
force-pushed
the
feat/extends-mounted-secret-api
branch
from
49f2752
to
e833f5d
Compare
Resolves: #3323