Skip to content

Optionally accept RELATED_IMAGE_KEYCLOAK as the operand coordinate - #24018

Merged
vmuzikar merged 1 commit into
keycloak:mainfrom
ASzc:issue_24017
Oct 17, 2023
Merged

Optionally accept RELATED_IMAGE_KEYCLOAK as the operand coordinate#24018
vmuzikar merged 1 commit into
keycloak:mainfrom
ASzc:issue_24017

Conversation

@ASzc

@ASzc ASzc commented Oct 16, 2023

Copy link
Copy Markdown
Contributor

This changes the quarkus configuration of the operator so that if the RELATED_IMAGE_KEYCLOAK environment variable is present, it will be accepted over the default, or the existing configuration environment variable of OPERATOR_KEYCLOAK_IMAGE

Closes #24017

@ASzc
ASzc requested review from a team as code owners October 16, 2023 14:08
@ghost ghost added the team/cloud-native label Oct 16, 2023
@vmuzikar
vmuzikar requested review from shawkins and vmuzikar October 16, 2023 14:22
shawkins
shawkins approved these changes Oct 16, 2023
View reviewed changes

@shawkins shawkins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks @ASzc

abstractj
abstractj previously approved these changes Oct 16, 2023
View reviewed changes
@ghost ghost added the flaky-test label Oct 16, 2023
This was referenced Oct 16, 2023
Closed
Closed
ghost
ghost reviewed Oct 16, 2023
View reviewed changes

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@ghost

ghost commented Oct 16, 2023

Copy link
Copy Markdown

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.x509.X509BrowserLoginTest#loginWithRevalidateCertEnabledCertIsTrusted

Keycloak CI - Java Distribution IT (windows-latest - temurin - 19)

org.openqa.selenium.TimeoutException: 
{"errorMessage":"URL 'https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?response_type=code&client_id=test-app&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Fmaster%2Fapp%2Fauth&state=4ba705b2-2641-42d5-aef3-91442c3e12d2&scope=openid' didn't load. Error: 'timeout'","request":{"headers":{"Accept-Encoding":"gzip","Connection":"Keep-Alive","Content-Length":"319","Content-Type":"application/json; charset=utf-8","Host":"localhost:30968","User-Agent":"selenium/3.14.0 (java windows)"},"httpVersion":"1.1","method":"POST","post":"{\n  \"url\": \"https:\\u002f\\u002flocalhost:8543\\u002fauth\\u002frealms\\u002ftest\\u002fprotocol\\u002fopenid-connect\\u002fauth?response_type=code\\u0026client_id=test-app\\u0026redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Fmaster%2Fapp%2Fauth\\u0026state=4ba705b2-2641-42d5-aef3-91442c3e12d2\\u0026scope=openid\"\n}","url":"/url","urlParsed":{"anchor":"","query":"","file":"url","directory":"/","path":"/url","relative":"/url","port":"","host":"","password":"","user":"","userInfo":"","authority":"","protocol":"","source":"/url","queryKey":{},"chunks":["url"]},"urlOriginal":"/session/58bfa5e0-6c37-11ee-a005-578c27ad18df/url"}}
Command duration or timeout: 0 milliseconds
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:67)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
...

Report flaky test

vmuzikar
vmuzikar reviewed Oct 17, 2023
View reviewed changes
Comment thread operator/src/main/resources/application.properties Outdated
mabartos
mabartos reviewed Oct 17, 2023
View reviewed changes

@mabartos mabartos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ASzc I've added one nitpick.

I assume a normal user will not have any use case to leverage the functionality as it's mainly related to the internal build process, right? I mean, the config does not have to be documented, right?

Comment thread operator/src/main/resources/application.properties Outdated
@ASzc
Optionally accept RELATED_IMAGE_KEYCLOAK as the operand coordinate
c3726b0 
This changes the quarkus configuration of the operator so that if the
RELATED_IMAGE_KEYCLOAK environment variable is present, it will be
accepted over the default value

Closes keycloak#24017
@ASzc ASzc mentioned this pull request Oct 17, 2023
Merged
@ASzc

ASzc commented Oct 17, 2023

Copy link
Copy Markdown
Contributor Author

Updated and rebased, I've filed a backport PR too, since this needs to land in 22.x today, ideally.

#24062

vmuzikar
vmuzikar approved these changes Oct 17, 2023
View reviewed changes

@vmuzikar vmuzikar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ASzc LGTM, thank you!

@vmuzikar
vmuzikar enabled auto-merge (squash) October 17, 2023 15:10
@vmuzikar
vmuzikar merged commit 359a946 into keycloak:main Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shawkins shawkins shawkins approved these changes
@vmuzikar vmuzikar vmuzikar approved these changes
@mabartos mabartos mabartos approved these changes
@abstractj abstractj abstractj left review comments
+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Operator operand env var does not conform to OSBS naming requirements

5 participants

@ASzc @abstractj @shawkins @vmuzikar @mabartos