Skip to content

Commit

Permalink
Expand proposal to include overview
Browse files Browse the repository at this point in the history
  • Loading branch information
@girstenbrei
girstenbrei committed Sep 8, 2023
1 parent d3bf042 commit e70cb7d
Showing 1 changed file with 176 additions and 14 deletions.
190 changes: 176 additions & 14 deletions docs-v2/design_proposals/consistent-templating.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,23 @@ cannot expand release name "{{ default \"latest\" .NAME }}": template: envTempla
To a user, this behavior seems confusing. Why can the `default` function be used in one field and not in another? Why are some values only available in `setValueTemplates` (e.g. `.IMAGE_*`)? Why does templating render `<no value>` in a field?

**Goals**

1. Provide an inherently consistent user experience of the golang templating language.\
This includes having the same templating functions available in all fields and
a consistent error behavior across fields.
2. Provide a consistent set of values to template with inside a specific pipeline stage.\
This includes having access to all template values available to the pipeline
stage in question.
3. Provide values to template with in a structured way to downstream pipeline stages.\
This includes having access to build information in deploy stages.
___

## Design

The design is split according to the goals set.

### Phase 1: Consistent templating UX
Currently, templating is mainly happening via
[`util.ExpandEnvTemplate(...)` @ util/env_template.go#43](https://github.com/GoogleContainerTools/skaffold/blob/bca77a4f2d421f487adf20c6b34af0daba08c2f7/pkg/skaffold/util/env_template.go#L43). It is a single unit of code execution,
you provided it with a template string and a map of variables, it gives you back
Expand All @@ -94,14 +107,8 @@ To solve this, provide a `util.Templater` object. This should be able to:
This can then be used by every location in code which needs templating.
A somewhat similar struct is already in use by [tag templating @ tag/env_template.go#30](https://github.com/GoogleContainerTools/skaffold/blob/bca77a4f2d421f487adf20c6b34af0daba08c2f7/pkg/skaffold/tag/env_template.go#L30)

### Out of Scope

1. Values available for templating stay the same
2. Templating stays limited to template the contents of fields (as opposed to support helm-like templating of complete files).

### Open Issues/Questions

Please list any open questions here in the following format:
#### Open Issues/Questions

**\<Should parsed templates be cached?\>** \
To be able to to do so effectively would
Expand All @@ -123,16 +130,171 @@ be set, making it a non-option.

Resolution: __Not Yet Resolved__


### Phase 2: Stage value consistency
Currently, not all fields in a single stage have access to the same values. E.g.
image build information is available in the helm deployer only in `setValueTemplates`
but not other templateable fields. \
As a user, this makes it difficult to remember which values are available to
which fields.

Although one might discuss the sensibility of having some information available in
some fields, the decision what is and what is not sensible in the current
setup should be left to the user.

To implement this, for every component using templating:

1. Centralize value instantiation to once at the start of each component execution.\
Those values can then be re-used during execution for all templates.
2. Implement instantiation consistency across components.\
E.g. all deployers should have the same values to template with available to them,
providing a user the same templating UX switching between different components
of the same pipeline stage.

This should implementation should only lift current existing restrictions,
allowing the use of values consistently across fields and components of the same
pipeline stage. All in al templating should feel more useful and uniform without
breaking any current existing functionality.

#### Open Issues/Questions

None.

### Phase 3: Downstream value passing
Currently, a pipeline stage downstream from another is responsible for computing
their own values to template with. This is e.g. done from `builds []graph.Artifact`
and especially useful to access build tags for this skaffold execution in
subsequent deploy and render stages.

Having each component responsible to compute those values on their own opens
up the implementation for inconsistencies. Centralizing the computation logic
of available template values can prevent that.

To do so, one can view those values available to a user as an API which should
have a specification. A draft specification can be the following:

```yaml
"$schema": https://json-schema.org/draft/2020-12/schema
type: object
properties:
Env:
description: |
All environment variables available to the skaffold process on startup are
available to the template user under the .Env key.
type: object
additionalProperties:
type: string
Config:
description: |
The configuration of skaffold contains some global values which are not
subject to templating. These are accessible to the templating user under
the .Config key.
type: object
properties:
name:
description: The name of the skaffold configuration running
type: String
labels:
description: Labels attached to this skaffold configuration
type: object
additionalProperties:
type: string
annotations:
description: Annotations attached to this skaffold configuration
type: object
additionalProperties:
type: string
additionalProperties: false
Builds:
description: |
In all stages after tagging, the information how builds where tagged is
available to the template user under the .Builds key.
type: array
items:
type: object
properties:
imageName:
description: The name of a build as referred to in the configuration
type: string
tag:
description: The name of the build as tagged by skaffold
type: string
additionalProperties: false
# Build information might not be available in all stages
required:
- Env
- Config
# TBD, see open questions
additionalProperties: false
```
#### Open Issues/Questions

**\<Do we need less or more values?\>** \
The above values are a proposal and motivated by ideas from looking at the current
source code and what one could do with it. They might miss out on some thing
while being overkill for others. One such open value would be if the current
running profile should be available.

Resolution: __Not Yet Resolved__

**\<Is the schema adequate?\>** \
The shape of available data as proposed above is inspired by accessing Helm
values. This might not be the most intuitive solution here.

Resolution: __Not Yet Resolved__

**\<Where to compute those values?\>** \
Where is the correct place in code to do this value computation?

Resolution: __Not Yet Resolved__

**\<Does the current available values stay?\>** \
Currently values are available in a different way (e.g. the `IMAGE_REPO_*` format).
This could be kept for backwards compatibility reasons but would introduce additional
maintenance and testing burden. Removing them is definitely a breaking change and
would therefore require a major version change.

Resolution: __Not Yet Resolved__


**\<Value clashing?\>** \
The root property naming convention is adhering to the helm naming convention.
This naming convention is different than the all-caps format usually used by
environment variables and the current variables. This whoever does not guarantee
uniqueness, as, of course, a user can already have an environment variable
e.g. named `Env`.

Resolution: __Not Yet Resolved__

## Implementation plan

1. Provide the `Templater` + appropriate functions & tests
2. Make helm deployer + renderer use the new templater as a first implementation
example. Provide a consistent set of values to template with for every field.
3. Switch all other deployers & renderers to the new templating logic
To keep PR size manageable, the implementation should be split into at least 3
parts following the three different design goals. These could be split further
into the sub-list items.

1. Consistent templating UX
1. Implement `templater` and switch only one component
2. Switch the rest to the new implementation to make them consistent
2. Stage value consistency
1. Centralize value instantiation for one component
2. Switch the rest to the new implementation to make them consistent
3. Downstream value passing
1. Implement objects (e.g. `.Env`, `.Config`, `.Builds`) one by one instead
of all of them at once.

## Integration test plan

Every test currently present should still pass. The implementation moves around
where things are instantiated and should support a superset of the current output.
For phase 1 & 2, functionality should only expand. All currently passing tests
should still pass. New code has to have associated tests with it.
This should ensure we don't brake current functionality as well as make new
code as solid as we can.

Add tests to test behavior of the `Templater` itself to ensure valid results.
For phase 3, centralizing the generation of those values should provide a point
of leverage for testing. This can be done by directly testing the generating
code or using a specification validation setup.

0 comments on commit e70cb7d

Please sign in to comment.