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

Support jsonnet as configuration source. #4855

Merged
merged 2 commits into from
Nov 11, 2020

Conversation

S-Bohn
Copy link
Contributor

@S-Bohn S-Bohn commented Oct 5, 2020

Adding a design proposal for adding Jsonnet support to write the configuration file.

The initial basic implementation can be found here: https://github.com/S-Bohn/skaffold/tree/jsonnet-impl-basic-support

@S-Bohn S-Bohn requested a review from a team as a code owner October 5, 2020 10:14
@S-Bohn S-Bohn requested a review from ilya-zuyev October 5, 2020 10:14
@gsquared94
Copy link
Contributor

Thanks for your initiative on this @S-Bohn. Can you split out the description of this PR into its own PR following our design proposal process.

@S-Bohn
Copy link
Contributor Author

S-Bohn commented Oct 6, 2020

Thanks for your initiative on this @S-Bohn. Can you split out the description of this PR into its own PR following our design proposal process.

I updated this MR to include the design proposal only.

@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #4855 (427f2eb) into master (68eab61) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4855   +/-   ##
=======================================
  Coverage   71.85%   71.86%           
=======================================
  Files         356      356           
  Lines       12215    12218    +3     
=======================================
+ Hits         8777     8780    +3     
  Misses       2786     2786           
  Partials      652      652           
Impacted Files Coverage Δ
pkg/skaffold/build/cache/retrieve.go 68.60% <0.00%> (+0.36%) ⬆️
pkg/skaffold/docker/client.go 81.70% <0.00%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68eab61...264efef. Read the comment docs.

@gsquared94 gsquared94 added the triage/discuss Items for discussion label Oct 7, 2020
@ilya-zuyev
Copy link
Contributor

Jsonnet config schema upgrades (skaffold fix) could be challenging.

@S-Bohn
Copy link
Contributor Author

S-Bohn commented Oct 8, 2020

Jsonnet config schema upgrades (skaffold fix) could be challenging.

I would even say pretty much impossible for the common case. For something like:

local v = 'skaffold/v2beta6';
local k = 'api'+'Version';
{
  [k]: v,
  kind: 'Config',
  metadata: {name: 'test'},
  deploy: {kubectl: {manifests: ['foo.yaml']}},
}

i can imagine that (somehow) tracking the ultimate source of a specific value to change through the interpretation steps, fiddle around with AST to change it, and output it's source again may actually work. But latest with calculated fields i don't see any reasonable solution.

So fix would output the very same as of today: the updated YAML.

Note that all the following files currently return the very same output for skaffold fix:

local v = 'skaffold/v2'+'beta6';
local k = 'api'+'Version';
{
  [k]: v,
  kind: 'Config',
  metadata: {name: 'test'},
  deploy: {kubectl: {manifests: ['foo.yaml']}},
}
.version: &foo |-
  skaffold/v2beta6

apiVersion: *foo
kind: Config
metadata:
  name: test
deploy:
  kubectl:
    manifests:
    - 'foo.yaml'
.hide-me: &a 
  apiVersion: skaffold/v2beta6
  kind: Config

<< : *a
metadata:
  name: test
deploy:
  kubectl:
    manifests:
    - 'foo.yaml'

So the schema upgrades already take place after the usual YAML preprocessing and don't return the original version.

@briandealwis
Copy link
Member

I think this is an interesting idea, not specifically for jsonnet but for the general approach of providing a "generator". It could allow experimenting with other config languages like cue, and help address problems reported by users where they want to use templating. I'd like to avoid having to link in jsonnet into Skaffold — Skaffold is big enough as it is — but maybe we could use a kubectl plugin-like mechanism where Skaffold could call out to named generators based either on the extension or an explicitly configured generator.

For example:

skaffold build -f skaffold.cue

would look for and pipe the file through a skaffold-cue generator if found.

skaffold build --generator my-templating-engine

would pipe the skaffold.yaml through the my-templating-engine generator.

People could get shell-like environment expansion with a non-robust generator like:

#!/bin/sh
# Use the shell as a preprocessor to expand environment variable references
# in stdin using a "here" document.
#!/bin/sh
(echo 'cat <<__EOF__'; cat; echo '__EOF__') | sh

@briandealwis
Copy link
Member

(And good point @ilya-zuyev and there's not much we can do about that.)

@IsaacPD IsaacPD removed the triage/discuss Items for discussion label Oct 26, 2020
Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Shelved however can be resumed later.

@tejal29 tejal29 merged commit 00cde43 into GoogleContainerTools:master Nov 11, 2020
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.

7 participants