-
Notifications
You must be signed in to change notification settings - Fork 4
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/productionize image #24
Conversation
add pre-commit in the project add a pre-commit gh action add multi arch steps change names disable load init labels re-order steps of workflows use dockerhub push image and fix wkflw fix ci update ci update ci remove redundant OCI labels as it's set by builder update README
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 had another look over the action.
The remaining files looked good / Maxim already left some comments.
gsed not present in the runner
Thanks @maxdanilov and @Langleu for the review. Currently, it is not a blocking action (so even if snyk reports CVEs, we will still be able to merge/release). I think it could help but as discussed with Lars, it could be a little overkilled. @maxdanilov we'd like to hear your opinion on this, then the PR should be ready to be merged |
@leiicamundi I don't have a strong opinion, but more in favor of keeping it light for now and drop it. |
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.
Looks good in general, thank you 🚀
Just leaving small final styling suggestions + discovered hiccup with the pre-commit hook when running it locally.
unified scripts naming with snake case fix pre-commit runs update README
Hello @maxdanilov, thank you for the review. I have implemented the suggestions and also extracted the Snyk scan into its own issue (#25). I am unable to reproduce the pre-commit bug. Could you please retry with the latest version of the repository? Otherwise, I will likely switch back to the Python script, as it seems to work without the need to consider the underlying platform. |
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.
looks good from my side.
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.
All good, just last suggestion for fixing the colored output of ls
in one of the scripts so the pre-commit hook works consistently for all users.
Hey @Langleu, @maxdanilov, thank you for the reviews! Now that everything is aligned, I'm merging the PR. We'll continue to keep track of the remaining items to process at https://github.com/orgs/camunda/projects/87/views/1?pane=issue&itemId=47441722. |
Pull Request Description:
This pull request addresses the need for a production-ready Camunda-flavored Keycloak Docker image that supports AWS IAM roles for Service Accounts (IRSA) out of the box. Currently, customers are required to create their own Keycloak Docker image for AWS with IRSA support, and this enhancement aims to simplify this process.
Problem Description:
Customers running Keycloak in AWS with IRSA face challenges in creating custom Docker images. This PR introduces a Camunda-flavored Keycloak image for both ARM and AMD architectures, facilitating easier deployment in AWS environments.
Proposed Solution:
camunda/keycloak
to accommodate multi-cloud support in the future (already implemented, repository is now public).Desired Outcome:
camunda/keycloak
.Implementation Details:
Implementation Details:
In addition to the previously mentioned implementation details, a dynamic test matrix is now generated for all Keycloak versions. Each version undergoes a suite of tests on GCP, AWS, and Docker Compose environments.
The testing process follows these steps:
Dynamic Test Matrix:
Semantic Versioning and Image Tagging:
keycloak-<version>-latest
: Represents the latest version.keycloak-<version>-<yyyy-mm-dd-xxx>
: Timestamped version for historical reference and traceability.latest
: Used if the version is the most recent.Publishing Process:
Pending Tasks:
PS: This PR will be squashed merged due to the number of commits