-
Notifications
You must be signed in to change notification settings - Fork 325
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
new resources: realm key aes, ecdsa, hmac, rsa, java keystore #582
Conversation
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Thanks for the PR @Vlad-Kirichenko, this is looking good so far. Let me know if you need help with anything. |
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Hello @mrparkers |
…keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
…keycloak into realm-keys
Sorry for the late response @Vlad-Kirichenko. Unfortunately, this is a feature of Keycloak that I'm not really familiar with, so I am not sure how to advise you on that test. |
That's all right. Then @mrparkers, what do you think if I will add step in |
Yeah I have no problem with that at all. If all this is doing is generating a file to mount to Keycloak's file system, you could also just generate it once and store it within |
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
…keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Hello @mrparkers, joining the conversation here for a quick question. @Vlad-Kirichenko is one of my team members, and the work done here is contributed by the company we work for. As a technical lead, I was able to have an approved list of OpenSource projects we use internally, so we could contribute back to these projects. Executive management approved but they would like these contributions linked to their brand. We would very much like to see a notion of this work logged somewhere, something like name of the company, hyperlinked to our website, and mentioning this PR and any later ones (e.g. #594). Can you live with this? If so, where would be the best place to put this? The README.md file? |
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
I'm not opposed to something like that, but I'm not sure that the README is the appropriate place for this. How about this - I'll make some changes to the changelog file, so that each release includes a "thank you" section that tags the individuals that contributed code towards that release. Once I've done that, you can submit a PR to update one of these entries to include something like "@Vlad-Kirichenko on behalf of Example Company". Does that sound like a good compromise? |
Okay, I've gone ahead and updated the changelog to include a list of each contributor under every release. Once I've cut a release that includes these changes, feel free to make a PR to this file to include a reference to the company that these changes were contributed on behalf of. |
Hello @mrparkers. |
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.
overall this looks really good, there's just a couple of issues I found
func TestAccKeycloakRealmKeystoreJava_basic(t *testing.T) { | ||
t.Parallel() | ||
|
||
skipIfEnvSet(t, "CI") // temporary while I figure out how to put java keystore file to keycloak container in CI |
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 could probably just mount the misc
directory as a volume on the running container and reference it that way. but I'm fine with leaving this as-is for now.
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.
Yes, I tried to to this(using the misc
dir) but the problem that I need keystore file inside keycloak instance, not in runner instance.
In the terraform manifest inside keystore
field I paste path for keycloak itself.
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
…der-keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
…keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
…keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
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.
Just a few more comments, sorry for the delay. we can get this merged and released ASAP as soon as these are addressed.
…keycloak into realm-keys
Signed-off-by: Vlad Kirichenko <v.kirichenko@napoleongames.be>
Guys, do you have any news about this topic? I would love to use it here, because we have some configurations that were done manually, and now I need to import the keys. |
…keycloak into realm-keys
Hello @mrparkers. can you review this PR and #599? Best regards, |
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 your patience, this looks great. Thanks again for your work on this! 🚀
@mrparkers when do you plan to release a new version with this resource available? :D |
@StuxxNet the company I work for is contributing 3 PRs, by way of my team member @Vlad-Kirichenko. Two of them are already merged, one is being finalised (#599). Are you OK that we get that last one in before a new release is created? |
Totally ok with it, @ringods. Just asking so I can plan myself to start developing here :) |
@StuxxNet it seems @mrparkers merged our latest PR and created that release: v3.5.0 😄 |
Thanks again for your patience @ringods and @Vlad-Kirichenko. Feel free to open a PR to update this line of the changelog: https://github.com/mrparkers/terraform-provider-keycloak/blob/c440ec199642debaa2a8f1cc0ec8e9a3a185e9bd/CHANGELOG.md?plain=1#L22 if you'd like to attribute these changes to the company you work for, as we discussed earlier. |
Thanks @Vlad-Kirichenko |
Hi @mrparkers,
This PR adds new resources to manage keystores. #569
Tried creating everything in one resource but decided to separate them.
That's why:
Work is still in progress, here's what needs to be done:
Best regards,
Vlad