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

Provisioning: Interpolate env vars in provisioning files #16499

Merged
merged 12 commits into from
Apr 24, 2019

Conversation

aocenas
Copy link
Member

@aocenas aocenas commented Apr 10, 2019

Closes: #12896

Allows $ENV_VAR or ${ENV_VAR} syntax in provisioning files. Uses https://golang.org/pkg/os/#Expand for the interpolation so does not allow any other syntax at the moment.

The interpolation is implemented as special values that can be used in the struct into which the data should be unmarshalled. Example:

type Data struct {
  Field StringValue `yaml:"field"` // StringValue instead of string
}

This allows better flexibility and can be reused, while specifying custom unmarshaling logic for each config object would have been tedious especially as you would have to reimplement lots of mapping logic that yaml lib gives you for free with this approach.

Other approach considered would be to do the interpolation directly on yaml string when read from file and only afterwards unmarshal it. This would allow using env vars also for field names or even interpolating a bigger pieces of the document which we did not want to allow.

TODO:

  • notifier configs?
  • env vars in dashboard json files? not doing in this PR
  • check some advanced yaml syntax and add tests for it.
  • some manual testing for the notifications


Convey("Should unmarshal variable nesting", func() {
doc := `
val:
Copy link
Member Author

Choose a reason for hiding this comment

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

Yaml does not seem to allow tabs and so these must be spaces which then creates odd indent.

@marefr
Copy link
Contributor

marefr commented Apr 11, 2019

This looks like an interesting approach and a good start. Great that you've opened a PR early.

Think @bergquist is more suitable reviewing this though given that he has built the provisioning feature.

@marefr marefr requested a review from bergquist April 11, 2019 08:44
@@ -1,30 +1,61 @@
package notifiers

import "github.com/grafana/grafana/pkg/components/simplejson"
Copy link
Member Author

Choose a reason for hiding this comment

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

Small refactor with the types here to align it with other provisioning types and makes the change to Value much more contained. Basically we have ConfigVx type that can be versioned and yaml file is unmarshalled into it. Afterwards there is Config type that all ConfigVx are mapped into which separates the multiple possible config versions from internal representation. This also means that changes to the config unmarshalling are only done here and does not permeate other parts of the code that needs to work with the data.

@aocenas aocenas marked this pull request as ready for review April 11, 2019 12:13
@aocenas
Copy link
Member Author

aocenas commented Apr 11, 2019

@bergquist This should work right now. Not sure about the dashboards json files whether they should have interpolation or not, but even they do we can do it in separate PR.

Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

Great work! Could you please update the docs in the PR as well?

pkg/services/provisioning/values/values.go Outdated Show resolved Hide resolved
@@ -30,33 +30,19 @@ Checkout the [configuration](/installation/configuration) page for more informat

### Using Environment Variables

All options in the configuration file (listed below) can be overridden
using environment variables using the syntax:
It is possible to use environment variable interpolation in all 3 provisioning config types. Allowed syntax
Copy link
Member Author

Choose a reason for hiding this comment

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

@bergquist Added the documentation here. I removed the old text because it was talking about using env vars for main configuration.ini and was duplicate from installation/configuration.md so it seemed out of place and confusing here.

Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

Looks good. I think it would make sense to add tests for this in https://github.com/grafana/grafana/blob/master/pkg/services/provisioning/dashboards/config_reader_test.go#L16 as well. What do you think?

@bergquist bergquist added this to the 6.2 milestone Apr 16, 2019
@aocenas
Copy link
Member Author

aocenas commented Apr 17, 2019

@bergquist I added some env vars into the test configs kinda randomly. I think that would be enough right? Or do you think a separate test case for each would be better?

Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

Great work!

I think this is ready to be merged :)

@aocenas
Copy link
Member Author

aocenas commented Apr 18, 2019

@bergquist Will wait a bit with merging this for #16559 as it should be easier to solve the merge conflicts that way.

@aocenas
Copy link
Member Author

aocenas commented Apr 24, 2019

@bergquist There are some small changes after merge with master. I added Raw attribute to values that holds the uninterpolated value for a better warning here.

Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

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

Raw makes total sense. Great work :)

@aocenas aocenas merged commit fcebd71 into master Apr 24, 2019
@aocenas aocenas deleted the env-vars-provisioning branch April 24, 2019 13:39
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 25, 2019
…into streaming-test-data-alt

* 'streaming-test-data-alt' of github.com:ryantxu/grafana:
  PanelChrome: delay state update while other panel is in fullscreen/edit
  TestData: don't reuse unsubbed stream workers
  PanelQueryState: give code a bit of air (space)
  PanelQuery: Minor refactoring
  sqlstore: use column name in order by (grafana#16583)
  user friendly guide (grafana#16743)
  Provisioning: Interpolate env vars in provisioning files (grafana#16499)
  admin: add more stats about roles (grafana#16667)
  Feature: Migrate Legend components to grafana/ui (grafana#16468)
  playlist: fix loading dashboards by tag (grafana#16727)
  CloudWatch: Use default alias if there is no alias for metrics (grafana#16732)
  Provisioning: Support FolderUid in Dashboard Provisioning Config (grafana#16559)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 25, 2019
* grafana/master:
  GraphLegendEditor: use stats picker rather than switches (grafana#16759)
  Feature: add cron setting for the ldap settings (grafana#16673)
  Build: Disables gosec until identified performance problems (grafana#16764)
  Chore: bump jQuery to 3.4.0 including prototype pollution vulnerability fix (grafana#16761)
  elasticsearch: add 7.x version support (grafana#16646)
  Provisioning: Add API endpoint to reload provisioning configs (grafana#16579)
  Config: Show user-friendly error message instead of stack trace (grafana#16564)
  Chore: Lowered implicit anys limit to 5954
  Feature: Enable React based options editors for Datasource plugins (grafana#16748)
  sqlstore: use column name in order by (grafana#16583)
  user friendly guide (grafana#16743)
  Provisioning: Interpolate env vars in provisioning files (grafana#16499)
  admin: add more stats about roles (grafana#16667)
  Feature: Migrate Legend components to grafana/ui (grafana#16468)
  playlist: fix loading dashboards by tag (grafana#16727)
  CloudWatch: Use default alias if there is no alias for metrics (grafana#16732)
  Provisioning: Support FolderUid in Dashboard Provisioning Config (grafana#16559)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 25, 2019
…MetricPanelCtrl

* grafana/master: (34 commits)
  Streaming: support streaming and a javascript test datasource (grafana#16729)
  GraphLegendEditor: use stats picker rather than switches (grafana#16759)
  Feature: add cron setting for the ldap settings (grafana#16673)
  Build: Disables gosec until identified performance problems (grafana#16764)
  Chore: bump jQuery to 3.4.0 including prototype pollution vulnerability fix (grafana#16761)
  elasticsearch: add 7.x version support (grafana#16646)
  Provisioning: Add API endpoint to reload provisioning configs (grafana#16579)
  Config: Show user-friendly error message instead of stack trace (grafana#16564)
  Chore: Lowered implicit anys limit to 5954
  Feature: Enable React based options editors for Datasource plugins (grafana#16748)
  sqlstore: use column name in order by (grafana#16583)
  user friendly guide (grafana#16743)
  Provisioning: Interpolate env vars in provisioning files (grafana#16499)
  admin: add more stats about roles (grafana#16667)
  Feature: Migrate Legend components to grafana/ui (grafana#16468)
  playlist: fix loading dashboards by tag (grafana#16727)
  CloudWatch: Use default alias if there is no alias for metrics (grafana#16732)
  Provisioning: Support FolderUid in Dashboard Provisioning Config (grafana#16559)
  Refactor: Make SelectOptionItem a generic type to enable select value typing (grafana#16718)
  docs: fix upgrade instructions
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 25, 2019
* grafana/master:
  Streaming: support streaming and a javascript test datasource (grafana#16729)
  GraphLegendEditor: use stats picker rather than switches (grafana#16759)
  Feature: add cron setting for the ldap settings (grafana#16673)
  Build: Disables gosec until identified performance problems (grafana#16764)
  Chore: bump jQuery to 3.4.0 including prototype pollution vulnerability fix (grafana#16761)
  elasticsearch: add 7.x version support (grafana#16646)
  Provisioning: Add API endpoint to reload provisioning configs (grafana#16579)
  Config: Show user-friendly error message instead of stack trace (grafana#16564)
  Chore: Lowered implicit anys limit to 5954
  Feature: Enable React based options editors for Datasource plugins (grafana#16748)
  sqlstore: use column name in order by (grafana#16583)
  user friendly guide (grafana#16743)
  Provisioning: Interpolate env vars in provisioning files (grafana#16499)
  admin: add more stats about roles (grafana#16667)
  Feature: Migrate Legend components to grafana/ui (grafana#16468)
  playlist: fix loading dashboards by tag (grafana#16727)
  CloudWatch: Use default alias if there is no alias for metrics (grafana#16732)
  Provisioning: Support FolderUid in Dashboard Provisioning Config (grafana#16559)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 25, 2019
* grafana/master: (31 commits)
  Streaming: support streaming and a javascript test datasource (grafana#16729)
  GraphLegendEditor: use stats picker rather than switches (grafana#16759)
  Feature: add cron setting for the ldap settings (grafana#16673)
  Build: Disables gosec until identified performance problems (grafana#16764)
  Chore: bump jQuery to 3.4.0 including prototype pollution vulnerability fix (grafana#16761)
  elasticsearch: add 7.x version support (grafana#16646)
  Provisioning: Add API endpoint to reload provisioning configs (grafana#16579)
  Config: Show user-friendly error message instead of stack trace (grafana#16564)
  Chore: Lowered implicit anys limit to 5954
  Feature: Enable React based options editors for Datasource plugins (grafana#16748)
  sqlstore: use column name in order by (grafana#16583)
  user friendly guide (grafana#16743)
  Provisioning: Interpolate env vars in provisioning files (grafana#16499)
  admin: add more stats about roles (grafana#16667)
  Feature: Migrate Legend components to grafana/ui (grafana#16468)
  playlist: fix loading dashboards by tag (grafana#16727)
  CloudWatch: Use default alias if there is no alias for metrics (grafana#16732)
  Provisioning: Support FolderUid in Dashboard Provisioning Config (grafana#16559)
  Refactor: Make SelectOptionItem a generic type to enable select value typing (grafana#16718)
  docs: fix upgrade instructions
  ...
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.

Support for environment variables in provisioning
3 participants