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

pkg/helm: add overrideValues to watches.yaml (with env expansion) #2325

Merged
merged 1 commit into from
Jan 3, 2020

Conversation

joelanford
Copy link
Member

@joelanford joelanford commented Dec 12, 2019

Description of the change:
Adds overrideValues field to Helm operator's watches.yaml file, which allows an operator author to specify a values map that is merged with (and overrides) CR spec values.

The values defined in overrideValues can use environment variable expansion (e.g. $MY_VAR or ${MY_VAR}) to use values from the environment.

Motivation for the change:
In order to support disconnected scenarios and automated operand image rebuilds (e.g. to fix a CVE) it is necessary

To Do

  • Update CHANGELOG
  • Update documentation to include information about overrideValues
  • Document that if environment variables are used in overrideValues, the container image should be built such that each environment variable is set to the chart's default value for that field.
  • Log a warning, add condition to CR status, and/or use a Kubernetes event to notify when CR values are overridden

@joelanford joelanford added kind/feature Categorizes issue or PR as related to a new feature. language/helm Issue is related to a Helm operator project labels Dec 12, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 12, 2019
Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

Docs and some tests and then look pretty good to me at first glance.


yaml "gopkg.in/yaml.v2"
"github.com/ghodss/yaml"
Copy link
Member

Choose a reason for hiding this comment

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

Is this a conscious change and why?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is. When gopkg.in/yaml.v2 unmarshals, the resulting map[string]interface{} child maps with type map[interface{}]interface{}. github.com/ghodss/yaml gets this right with map[string]interface{} all the way down.

Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

See the previous comment

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 19, 2019
@joelanford joelanford changed the title WIP: pkg/helm: add overrideValues to watches.yaml (with env expansion) pkg/helm: add overrideValues to watches.yaml (with env expansion) Dec 19, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 19, 2019
Copy link
Contributor

@jmccormick2001 jmccormick2001 left a comment

Choose a reason for hiding this comment

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

lgtm

@joelanford joelanford merged commit 43cf58d into operator-framework:master Jan 3, 2020
@joelanford joelanford deleted the helm-image-env branch January 3, 2020 20:19
camilamacedo86 added a commit to operator-framework/operator-sdk-samples that referenced this pull request Feb 19, 2020
* upgrade helm to use SDK 0.14.1 by:
* changes applied by in roles, crd and watch by comparing the project with an empty new helm project created with 0.14.1. 
* the watch was changed because with it no longer equired to inform the full path. See: (operator-framework/operator-sdk#2325)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/helm Issue is related to a Helm operator project size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants