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

Add support for template_driver in composefiles #1100

Closed
wants to merge 1 commit into from

Conversation

vdemeester
Copy link
Collaborator

@vdemeester vdemeester commented May 31, 2018

This maps the --template-driver flag on secret and config creation.

See #896 (and upstream moby/moby#33702)

  • probably need some test
  • as for the initial flag, need some documentation too

Signed-off-by: Vincent Demeester vincent@sbr.pm

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

SGTM but it definitely needs some tests 😄

@vdemeester vdemeester force-pushed the compose-template-driver branch 2 times, most recently from e7957c3 to 17a83b9 Compare June 27, 2018 11:56
@vdemeester
Copy link
Collaborator Author

@silvin-lubecki added some test
cc @thaJeztah

@thaJeztah
Copy link
Member

Linting failure;

Successfully tagged cli-linter:13932
cli/compose/loader/loader_test.go:1::warning: file is not gofmted with -s (gofmt)
Exited with code 1

This maps the `--template-driver` flag on secret and config creation.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
},
},
}
assert.DeepEqual(t, config, expected, cmpopts.EquateEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

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

You can simplify the test:

assert.Assert(t, is.Equal(config.Configs["config"].TemplateDriver, "config-driver"))
assert.Assert(t, is.Equal(config.Secrets["secret"].TemplateDriver, "secret-driver"))

@overmike
Copy link

Will this be included in 18.09 release?

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

Not sure if this can still make it into 18.09. If it doesn't, this change needs to be moved to a 3.8 schema for the compose file.

@cpuguy83
Copy link
Collaborator

cpuguy83 commented Sep 8, 2018

Is there another driver other than the built-in go one?
What's the reasoning for wanting this anytime soon?

@thaJeztah
Copy link
Member

Good point; don't think there's another driver yet; otoh, isn't it required to set a driver in order to use templating? (will have to check)

@thaJeztah
Copy link
Member

To come back to my previous comment;

Templating controls whether and how to evaluate the config payload as a template. If no driver is set, no templating is used.

So, yes, we need this option to enable templating, otherwise no templating will be applied.

I see this needs a rebase though (and would now have to be moved to docker-compose schema 3.8

@thaJeztah
Copy link
Member

Carried in #1746

@vdemeester
Copy link
Collaborator Author

Thanks @thaJeztah 😽

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

Successfully merging this pull request may close these issues.

6 participants