-
Notifications
You must be signed in to change notification settings - Fork 120
java-openliberty: use ENV variables for security credentials #770
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking we might as well do the same thing for java-microprofile. It's already otherwise in-sync, so might be better to keep it in sync, even though it's deprecated.... After all java-microprofile previously had this capability (before we crippled it in 1Q), so it's not like we're adding something brand new to the deprecated stack.
incubator/java-openliberty/README.md
Outdated
name: mySecret | ||
``` | ||
|
||
This will set environment variables in the deployment called `STACK_USERNAME` and `STACK_PASSWORD` which map to the username and password of the "mySecret" Secret respectively. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of the "mySecret" Secret
=>
of mySecret
, respectively.
(You mentioned it's a Secret enough I think).
@@ -3,6 +3,8 @@ | |||
<feature>microProfile-3.2</feature> | |||
</featureManager> | |||
|
|||
<quickStartSecurity userName="${stack.username}" userPassword="${stack.password}" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great from the perspective of something new, but we changed the "contract" on configDropins/defaults mid-stream.
Is this OK? Can you maybe spell out some more thought about why this shouldn't be a concern? Is there any way to do this with a file named: configDropins/defaults/quick-start-security.xml
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean by "contract." Is the concern that we had a file called quick-start-security.xml and now we dont? Or that we were removing it and now we aren't? Given that this is just a change to the template, not sure if it matters.
262c8aa
to
d5c150e
Compare
If this is steering us towards a 0.3 update because of back-compatibility with already-init'd 0.2 projects then I think we should slow down and consider doing this via doc, especially since we've talked (maybe just privately though?) about using an Open Liberty service token. I'm going to close this PR for now, while we discuss. |
Checklist:
[ x] Read the Code of Conduct and Contributing Guidelines.
[ x] Followed the commit message guidelines.
[ x] Stack adheres to Appsody stack structure.
Modifying an existing stack:
stack.yaml
appsody build
Related Issues:
Fixes #750