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

Read os env vars #158

Open
wants to merge 9 commits into
base: stable
Choose a base branch
from
Open

Read os env vars #158

wants to merge 9 commits into from

Conversation

DamandeepToor
Copy link
Contributor

@DamandeepToor DamandeepToor commented Sep 8, 2022

Description of the change

Adding environment variables to be read by the Gateway

Benefits

Example to add environment variables as secret variables and read via Gateway context variable syntax in Policy and in pre-selected configuration fields (e.g. JDBC Connection URL)

Drawbacks

Applicable issues

  • fixes #

Additional information

Checklist

  • [] Chart version bumped in Chart.yaml according to semver.
  • [] Variables are documented in the README.md
  • [] Title of the PR starts with chart name (e.g. [charts/gateway])
  • [] If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

@Gazza7205
Copy link
Contributor

The repo that you are trying to merge is 258 commits behind stable.

Do we need an additional template for this? AdditionalEnv/Secrets can already be configured here - https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/values.yaml#L453-L461

For production environments we advise against exposing secrets in values.yaml - it's best to make use of existingSecrets where applicable

@DamandeepToor
Copy link
Contributor Author

@Gazza7205 , oh may be you are right, let me merge the stable to this PR and add the env to https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/values.yaml#L453-L461

@Gazza7205
Copy link
Contributor

Gazza7205 commented Sep 16, 2022

@Gazza7205 , oh may be you are right, let me merge the stable to this PR and add the env to https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/values.yaml#L453-L461

Do we need an additional template for this given that we already have mechanisms in place?

Sounds like we just need to update the readme.

# Conflicts:
#	charts/gateway/README.md
#	charts/gateway/templates/_helpers.tpl
#	charts/gateway/templates/deployment.yaml
#	charts/gateway/values.yaml
@DamandeepToor
Copy link
Contributor Author

@Gazza7205 , oh may be you are right, let me merge the stable to this PR and add the env to https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/values.yaml#L453-L461

Do we need an additional template for this given that we already have mechanisms in place?

Sounds like we just need to update the readme.

Updated ReadMe and revert other changes.

@Gazza7205
Copy link
Contributor

@Gazza7205 , oh may be you are right, let me merge the stable to this PR and add the env to https://github.com/CAAPIM/apim-charts/blob/stable/charts/gateway/values.yaml#L453-L461

Do we need an additional template for this given that we already have mechanisms in place?
Sounds like we just need to update the readme.

Updated ReadMe and revert other changes.

It seems this readme is also from an old branch - could you review your changes and make sure they are against the latest stable commit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants