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

Conditional execution of steps with if #220

Closed
drevell opened this issue Sep 21, 2023 · 0 comments · Fixed by #229
Closed

Conditional execution of steps with if #220

drevell opened this issue Sep 21, 2023 · 0 comments · Fixed by #229
Labels
enhancement New feature or request

Comments

@drevell
Copy link
Contributor

drevell commented Sep 21, 2023

TL;DR

We could add the ability to conditionally execute one or more template steps using a CEL predicate.

Detailed design

There are two possibilities we can think of for how this might work. There could be an `if` field on every action, or there could be a new action type `if`.

Design 1: an `if` field on every action


inputs:
  - name: 'include_auth0'

steps:
  - action: 'include'
    if: 'bool(include_auth0)'
    desc: 'conditionally include auth0 integration'
    params: 
      paths: ['auth0']

Design 2: a new action type for if that has a list of steps:

inputs:
  - name: 'include_auth0'

steps:
 - action: `if`
   params:
     then:
       - action: 'include'
          expr: 'bool(include_auth0)'
          desc: 'conditionally include auth0 integration'
          params: 
            paths: ['auth0']


### Alternatives considered

_No response_

### Additional information

_No response_
@drevell drevell added the enhancement New feature or request label Sep 21, 2023
drevell added a commit that referenced this issue Oct 10, 2023
…o present all the pieces at the same time.

This PR creates a system for supporting YAML files using different `api_version`s from a single binary. This is implemented by having a separate set of structs for each `(kind,api_version)` pair. Each api_version can change the semantics or existence of YAML fields. The structs are stored in `templates/model/$KIND/$APIVERSION`.

Instructions for adding a new `api_version` are in `templates/model/README.md`, that might be a good starting point for review.

Old versions are automatically converted to new versions before being used: old structs define an Upgrade() method that define how to upgrade them.

There's a registry of `api_version`s in decode.go that defines which versions exist and which structs are associated with each.

To demonstrate that this all actually works, this PR creates a new API version `v1beta1`. The only change in this new version is adding the optional `if` field for each step in spec.yaml (#220).

Alternatives considered to try to avoid this huge copy-pasting of structs:
- Struct embedding to reduce copy-pasta: this would expose a more complicated API to the rest of the code that has to use the structs. It makes creating a new version more painful. It also triggers golang/go#15924 since we use `reflect.StructOf`.
- Use github.com/mitchellh/mapstructure` to avoid so much double-parsing: we'd lose YAML position information, and our users really appreciate getting line numbers on their error messages.
- Use `switch(apiVersion)` within a single struct instead of copy-pasting structs: high risk of accidentally changing the behavior of old API versions and generally creating fragile code.

I've marked files that are mostly copy-pasted so you can mostly skip reviewing them
drevell added a commit that referenced this issue Oct 10, 2023
Apologies for the large PR, but I think in this case it's important to
present all the pieces at the same time.

This PR creates a system for supporting YAML files using different
`api_version`s from a single binary. This is implemented by having a
separate set of structs for each `(kind,api_version)` pair. Each
api_version can change the semantics or existence of YAML fields. The
structs are stored in `templates/model/$KIND/$APIVERSION`.

Instructions for adding a new `api_version` are in
`templates/model/README.md`, that might be a good starting point for
review.

Old versions are automatically converted to new versions before being
used: old structs define an Upgrade() method that define how to upgrade
them.

There's a registry of `api_version`s in decode.go that defines which
versions exist and which structs are associated with each.

To demonstrate that this all actually works, this PR creates a new API
version `v1beta1`. The only change in this new version is adding the
optional `if` field for each step in spec.yaml (#220).

Alternatives considered to try to avoid this huge copy-pasting of
structs:
- Struct embedding to reduce copy-pasta: this would expose a more
complicated API to the rest of the code that has to use the structs. It
makes creating a new version more painful. It also triggers
golang/go#15924 since we use
`reflect.StructOf`.
- Use http://github.com/mitchellh/mapstructure to avoid so much
double-parsing: we'd lose YAML position information, and our users
really appreciate getting line numbers on their error messages.
- Use `switch(apiVersion)` within a single struct instead of
copy-pasting structs: high risk of accidentally changing the behavior of
old API versions and generally creating fragile code.
 
I've marked files that are mostly copy-pasted so you can mostly skip
reviewing them
drevell added a commit that referenced this issue Oct 11, 2023
Also: fix a bug where an error should have been returned but wasn't.

Fixes #220 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

1 participant