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

Feature/oc config #36

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

shokri-navid
Copy link

Contributing a Chart / update to an existing Chart

  • Run helm lint on the chart dir.
  • (Update) Bump the Chart.yaml version before merging, to release it as a new version.
  • (Update) If the PR includes new configurable parameters in the chart's values.yaml. Add documentation in the appropiate README.

Copy link
Contributor

@michielbdejong michielbdejong 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 so far! But I think it would be useful if it were even more configurable.
You created the option to switch between OC-10 backend and default backend.
It would be good if you could also make endpoint and shared_secret configurable, so that they are no longer hardcoded?

@shokri-navid
Copy link
Author

Ok, l will work on it and change will be made ASAP

@michielbdejong
Copy link
Contributor

As discussed just now, the shared_secret should be in Config -> Secrets at the Kubernetes level

@shokri-navid
Copy link
Author

@michielbdejong
I have no Idea About how can I inject the shared token from secret into the map config. because the secret is visible from the application execution process as an env variable, while the shared token is part of toml configuration which is hard coded

@michielbdejong
Copy link
Contributor

Ah, hm, so then we would need to run a bash script that edits revad.toml before revad starts up, or we should change revad to take it from env instead. Let's leave it as it is for now and then we can revisit this later!

@michielbdejong
Copy link
Contributor

CC @dagl

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

Successfully merging this pull request may close these issues.

3 participants