-
Notifications
You must be signed in to change notification settings - Fork 8
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
[#242] Create custom keycloak image for testing #371
Conversation
Rebuild image again for sanity check. Take out of draft if good. |
Builds good. Any future config changes should likely only see the |
I think we can use #242 for this. |
242 feels good and right. |
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.
Let's include words about the asymmetric keys in the README too.
Include as much info that seems helpful to the developer as possible.
can make a test/README.md to keep it scoped, if that seems a good idea. |
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.
Couple of minor comments. Solid Dockerfile
Suggestions on the README would be appreciated! |
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.
My README suggestion is basically... pretend Alan is asking you how to run these tests. What are you going to tell me to type to get the tests to run? I feel that would be very helpful.
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.
Thanks for the write-up - looking good
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 think this is in a good spot and ready for squashing. Maybe wait for one other opinion first.
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.
We're very close.
I think one more pass over it and this will be ready.
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.
Squash it!
f12e865
to
61cf005
Compare
Squashed. I think we should move, or add, the commit message to the keycloak readme. |
You're referring to the part about the secret keys and 10 years? I agree that should be captured in a README. Please do that. |
Added. |
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.
Once you address that final review comment, squash and pound it.
This gives us a good foundation to work with.
4ff5fe9
to
df18812
Compare
Squashed and pounded! |
go go go! |
you want the 10y and 'remember' in the commit message? |
Would you prefer a more general reminder in the commit, or none at all? More general: Or the previous with the last sentence dropped? |
i don't think the commit message makes sense for that information. just remove it i think. i see it in the README - which does make sense. |
df18812
to
aef591c
Compare
Removed the commit message. |
The environment should start the same on every standup by using the realm export. Asymmetric keys are only good for 10 years, I think. Remember to update before then!