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

Helm - Juice Shop - Custom environment variables and application config.yml #944

Merged
merged 7 commits into from
Feb 1, 2022

Conversation

fbuchmeier-abi
Copy link
Contributor

Description

Added two new options to the Juice Shop Helm Chart:

  • customConfig allows passing in a custom config file in yaml format
  • containerEnv allows setting custom container environment variables (for the juice-shop container), e.g. to use a different config file

Quick question, which license header to I have to add to the new configmap.yaml and what implications does this have?

Thanks!

Checklist

  • [ x ] Test your changes as thoroughly as possible before you commit them. Preferably, automate your test by unit/integration tests.

…config yml

Signed-off-by: florian.buchmeier@audi.de <florian.buchmeier@audi.de>
…ent variables and application config yml

Merge in OSSC/securecodebox-securecodebox from develop to main

* commit '5cab41e8f2d44396cb9fb2b6387a855e1013200c':
  added option to specify custom environment variables and application config yml
@J12934
Copy link
Member

J12934 commented Jan 27, 2022

Hi @fbuchmeier-abi thanks for the contribution looks good 👍

Not a lawyer so can't give you any legal guidance but my understanding is:

  • The License header should match the copyright field from the License file: https://github.com/secureCodeBox/secureCodeBox/blob/main/LICENSES/Apache-2.0.txt#L61
  • The license header is more of a precaution in case somebody copies / takes a single file out of the context of the repo
  • The sign-off flag on the commit gives the repo owner the (copy?)right to potentially use the code in a context outside the current Apache 2.0 license, e.g. release a new version of the SCB under MIT license without having to collect the signature of all previous contributors.

I'd like to change the copyright holder in the future to some "entity" which isn't just "iteratec", like "secureCodeBox core team" but not sure what the entails legally so can't promise anything 🤞

@J12934 J12934 added the enhancement New feature or request label Jan 27, 2022
Copy link
Member

@J12934 J12934 left a comment

Choose a reason for hiding this comment

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

Some small suggestions to properly include the descriptions / comments in side the helm docs page shown on artificthub.io.

(These are automatically generated using: https://github.com/norwoodj/helm-docs)

demo-targets/juice-shop/values.yaml Outdated Show resolved Hide resolved
demo-targets/juice-shop/values.yaml Outdated Show resolved Hide resolved
fbuchmeier-abi and others added 2 commits January 27, 2022 15:47
Co-authored-by: Jannik Hollenbach <jannik@hollenbach.de>
Co-authored-by: Jannik Hollenbach <jannik@hollenbach.de>
@fbuchmeier-abi
Copy link
Contributor Author

fbuchmeier-abi commented Jan 31, 2022

@J12934 thank you for the suggestions and the explanation. I need to check with my coworker first if the license header is OK for us. I'll update you once this is done.

Regards,
Florian.

Signed-off-by: florian.buchmeier@audi.de <florian.buchmeier@audi.de>
Merge in OSSC/securecodebox-securecodebox from license-header to main

* commit '78149d2c78eb2482f437cdd3d64402743f3a56eb':
  Added License Header
@fbuchmeier-abi
Copy link
Contributor Author

@J12934 I've updated the PR with the license header as requested: 78149d2

Anything else I can do?

Regards,
Florian.

@J12934
Copy link
Member

J12934 commented Feb 1, 2022

Perfect, thank you 🚀

@J12934 J12934 merged commit 59730fc into secureCodeBox:main Feb 1, 2022
@J12934
Copy link
Member

J12934 commented Feb 1, 2022

Tested this out works great 👍

One thing I've noticed is that you'll have to pretty much the entire default config file to have all required config values.
This is "normally" not required in Juice Shop as the customConfig get merged with the default config. Any values not overwritten will just use the default config. This doesn't work here as the volumeMount overwrites the entire config directory.

Was this intentional @fbuchmeier-abi ? Otherwise we could configure the mount of the custom config file to just write the one file without replacing the complete config directory.

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

Successfully merging this pull request may close these issues.

None yet

2 participants