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

feat(helm): Add Files.AsSecrets, Files.AsConfig, path functions #1666

Merged
merged 6 commits into from Dec 13, 2016

Conversation

andrewstuart
Copy link
Contributor

@andrewstuart andrewstuart commented Dec 9, 2016

This PR adds a number of useful utilities for working with files and file paths.

  • Several of the StdLib path functions have been exposed to templates for working with file paths
  • The Files type learned to serialize itself to valid YAML maps for both config maps and secrets via Files.AsConfig() and Files.AsSecrets()

A few possible edits I was unsure on:

  • AsConfig() and AsSecrets() could instead return maps intended to be passed to toYaml or toJson(does that exist?) to support either use case. I chose YAML because I think that's probably the most commonly used language, but I know there are people out there using JSON.
  • This could be more composable. For example, {{ Files.Glob "secrets/*" | secretMap | flattenDir | toYaml }}.
  • I'm not attached to the method names at all. There may be more semantic/evocative names for what these methods do. I just didn't want to overdo it with ToKubeConfigMapYAMLKeys() or some such nonsense. 😄

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 9, 2016
@k8s-reviewable
Copy link

This change is Reviewable

@andrewstuart andrewstuart changed the title Add Files.AsSecrets, Files.AsConfig, path functions feat(helm): Add Files.AsSecrets, Files.AsConfig, path functions Dec 11, 2016
// Add some extra functionality
extra := template.FuncMap{
"toYaml": chartutil.ToYaml,
"base": path.Base,
Copy link
Member

Choose a reason for hiding this comment

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

I guess my preference would be to have these path functions rolled into Sprig. We try to only put "special" functions here. If you put them in Sprig, I'll roll another Sprig release ASAP, too.

@technosophos
Copy link
Member

Other than discussing the possibility of moving the path functions to Sprig, I am on board with doing this.

@technosophos
Copy link
Member

I don't suppose I can coerce you into also doing this one too, could I? ;-) #1548

@technosophos
Copy link
Member

Manually tested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants