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.

Sign up for GitHub

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

Jump to bottom

Add driver field to top-level secret object #1783

Merged
merged 3 commits into from
Apr 12, 2019
Merged

Add driver field to top-level secret object #1783

merged 3 commits into from
Apr 12, 2019

Conversation

sirlatrom
Copy link
Contributor

- What I did
Added support for specifying a driver for a secret in a compose file when performing a docker stack deploy.

- How I did it
Added a driver field to the FileObjectConfig in cli/compose/types/types.go.

- How to verify it
make -f docker.Makefile binary and run the repro case in #1782.

- Description for the changelog

Added support for secret drivers in docker stack deploy

- A picture of a cute animal (not mandatory but encouraged)
image

@codecov-io
Copy link

codecov-io commented Mar 28, 2019 •
edited
Loading

Codecov Report

Merging #1783 into master will decrease coverage by 0.04%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master    #1783      +/-   ##
==========================================
- Coverage   56.31%   56.27%   -0.05%     
==========================================
  Files         308      308              
  Lines       21391    21416      +25     
==========================================
+ Hits        12047    12051       +4     
- Misses       8465     8482      +17     
- Partials      879      883       +4

@sirlatrom sirlatrom marked this pull request as ready for review March 28, 2019 14:10
@sirlatrom
Copy link
Contributor Author

I see a similar miniscule reduction in code coverage in the analogous PR #1746, so I hope that is not going to block this PR from being accepted.

@sirlatrom sirlatrom mentioned this pull request Mar 28, 2019
Closed
thaJeztah
thaJeztah requested changes Mar 28, 2019
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Had a quick look at the changes, and left some comments 🤗

cli/command/secret/create.go Outdated Show resolved Hide resolved
cli/compose/schema/data/config_schema_v3.8.json Show resolved Hide resolved
cli/compose/loader/loader.go Outdated Show resolved Hide resolved
cli/compose/schema/data/config_schema_v3.8.json Show resolved Hide resolved
@sirlatrom
Copy link
Contributor Author

@thaJeztah I have rebased and squashed as requested, PTAL.

@thaJeztah
Copy link
Member

ping @silvin-lubecki @vdemeester perhaps you could have a look as well? 🤗

@thaJeztah
Copy link
Member

note this will conflict with #1781 (which is targeted for 19.03), this will have to be rebased, and the bindata will have to be re-generated after that is merged

@sirlatrom
Copy link
Contributor Author

note this will conflict with #1781 (which is targeted for 19.03), this will have to be rebased, and the bindata will have to be re-generated after that is merged

I'll stand by to rebase and regenerate bindata once that is merged.

@sirlatrom sirlatrom force-pushed the stack_compose_secret_driver branch from 357c438 to ed838bf Compare April 5, 2019 10:01
thaJeztah
thaJeztah approved these changes Apr 5, 2019
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

ping @vdemeester @tiborvass @silvin-lubecki

vdemeester
vdemeester approved these changes Apr 5, 2019
View reviewed changes
Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@sirlatrom
Copy link
Contributor Author

@thaJeztah Since #1781 is not merged yet, I assume the regenerating of bindata will happen in that PR?

@thaJeztah
Copy link
Member

Spoke with @dperny and he's ok doing a rebase/regenerate of the spec, so let's merge 🎉

@thaJeztah thaJeztah merged commit 8b9cdab into docker:master Apr 12, 2019
@GordonTheTurtle GordonTheTurtle added this to the 19.03.0 milestone Apr 12, 2019
@sirlatrom sirlatrom deleted the stack_compose_secret_driver branch April 12, 2019 16:16
@thaJeztah thaJeztah mentioned this pull request Jun 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@vdemeester vdemeester vdemeester approved these changes

Assignees
No one assigned
Labels
impact/changelog impact/compose-file status/2-code-review
Projects
None yet
Milestone
19.03.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sirlatrom @codecov-io @thaJeztah @vdemeester @GordonTheTurtle