Skip to content
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

Removal of LGPL-2.1 licensed org.jboss.metadata.jboss-metadata-web dependency for CNCF compliance #24715

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

abstractj
Copy link
Contributor

@abstractj abstractj commented Nov 13, 2023

Closes #24714

Relates to: #24686

Before we go ahead with the merge, I want to get the approval from the @keycloak/cloud-native to ensure that this change won't break anything. I've already tried it out, which you can see here: keycloak-poc/keycloak#2. It seems that all the checks are passing in the pipeline.

If the team realizes that's not possible to remove we can close the proposed change here.

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

Seems safe to remove from my perspective. Tests pass, metrics and health work.

@shawkins
Copy link
Contributor

Looks good to me as well, but I do see some upstream usage via the intermediate undertow dependency https://github.com/search?q=repo%3Aquarkusio%2Fquarkus%20org.jboss.metadata&type=code - would it be possible for the quarkus folks to clarify if this dependency isn't needed specifically for micrometer. If that's the case, then they could exclude it upstream as well.

@abstractj
Copy link
Contributor Author

Looks good to me as well, but I do see some upstream usage via the intermediate undertow dependency github.com/search?q=repo%3Aquarkusio%2Fquarkus%20org.jboss.metadata&type=code - would it be possible for the quarkus folks to clarify if this dependency isn't needed specifically for micrometer. If that's the case, then they could exclude it upstream as well.

@shawkins @vmuzikar I chatted with Alexey from the Quarkus team today, and it turns out we don't really need that dependency unless we're using web.xml or jboss-web.xml. Looks like that's not the case for us, so we should be all set to move forward.

Copy link

@ghost ghost left a comment

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
Copy link

ghost commented Nov 17, 2023

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.ui.account2.ApplicationsTest#navigationTest

Keycloak CI - Account Console IT (chrome)

org.openqa.selenium.NoSuchElementException: 
no such element: Unable to locate element: {"method":"css selector","selector":"#username"}
  (Session info: headless chrome=118.0.5993.117)
For documentation on this error, please visit: http://seleniumhq.org/exceptions/no_such_element.html
Build info: version: '3.14.0', revision: 'aacccce0', time: '2018-08-02T20:19:58.91Z'
...

Report flaky test

org.keycloak.testsuite.account.AccountRestServiceTest#updateConsentForClient

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

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.keycloak.testsuite.account.AccountRestServiceTest.updateConsentForClient(AccountRestServiceTest.java:1256)
...

Report flaky test

@vmuzikar
Copy link
Contributor

@abstractj Thanks for looking into it further. Seems it's safe to merge.

…pendency for CNCF compliance

Signed-off-by: Bruno Oliveira da Silva <bruno@abstractj.com>

Closes keycloak#24714
@ghost
Copy link

ghost commented Nov 21, 2023

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.account.AccountRestServiceTest#updateConsentForClientWithPut

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

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.keycloak.testsuite.account.AccountRestServiceTest.updateConsentForClientWithPut(AccountRestServiceTest.java:1377)
...

Report flaky test

org.keycloak.testsuite.account.AccountRestServiceTest#createConsentForClientWithPut

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

java.lang.AssertionError: 
type
Expected: is "GRANT_CONSENT"
     but: was "UPDATE_CONSENT"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

org.keycloak.testsuite.account.AccountRestServiceTest#createConsentForClient

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

java.lang.AssertionError: 
type
Expected: is "GRANT_CONSENT"
     but: was "UPDATE_CONSENT"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

org.keycloak.testsuite.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportNFC

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=bd5dcb8e-a620-4926-814a-8b978f2c882c&response_mode=fragment&response_type=code&scope=openid&nonce=c78a92a5-62d4-4ac8-93e7-c601e1fbd35d&code_challenge=wQb7C_Yg61f2vCg7KNCdyHspfZ2xUH1ZZTW5VZnQ034&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor635.invoke(Unknown Source)
...

Report flaky test

Copy link

@ghost ghost left a comment

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

Copy link

@ghost ghost left a comment

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
Copy link

ghost commented Nov 22, 2023

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.webauthn.account.WebAuthnTransportLocaleTest#localizationTransportNFC

Keycloak CI - WebAuthn IT (chrome)

java.lang.AssertionError: Expected OIDCLogin but was  (https://localhost:8543/auth/realms/test/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Flocalhost%3A8543%2Fauth%2Frealms%2Ftest%2Faccount%2F%23%2Fsecurity%2Fsigningin&state=bd5dcb8e-a620-4926-814a-8b978f2c882c&response_mode=fragment&response_type=code&scope=openid&nonce=c78a92a5-62d4-4ac8-93e7-c601e1fbd35d&code_challenge=wQb7C_Yg61f2vCg7KNCdyHspfZ2xUH1ZZTW5VZnQ034&code_challenge_method=S256)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.page.AbstractPage.assertCurrent(AbstractPage.java:110)
	at jdk.internal.reflect.GeneratedMethodAccessor635.invoke(Unknown Source)
...

Report flaky test

ShefeeqPM pushed a commit to ShefeeqPM/keycloak that referenced this pull request Jan 27, 2024
…pendency for CNCF compliance (keycloak#24715)

Signed-off-by: Bruno Oliveira da Silva <bruno@abstractj.com>

Closes keycloak#24714

Signed-off-by: ShefeeqPM <86718986+ShefeeqPM@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removal of LGPL-2.1 licensed org.jboss.metadata.jboss-metadata-web dependency for CNCF compliance
4 participants