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

7.8.0 oauth fix v2 #173

Open
wants to merge 55 commits into
base: 7.8.0-post
Choose a base branch
from
Open

7.8.0 oauth fix v2 #173

wants to merge 55 commits into from

Conversation

ethaden
Copy link
Member

@ethaden ethaden commented Jan 13, 2025

Description

This pull requests utilizes init containers for setting up resources in the OAuth demo in order to reach a higher level of automation, remove some glitches that had been identified and improve the overall user experience of the demo by making it more robust.

Author Validation

The cp-all-in-one-security demo has been carefully checked by the requester. The reviewer should check the demo again, though.

Reviewer Tasks

Please check that all parts of the cp-all-in-one-security demo are working as expected.

ConfluentSemaphore and others added 30 commits September 3, 2024 06:39
Merge Conflict Resolution (from 7.8.x to 7.9.x)
Merge Conflict Resolution (from 7.9.x to master)
@ethaden ethaden requested a review from a team as a code owner January 13, 2025 14:09
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@ethaden
Copy link
Member Author

ethaden commented Jan 13, 2025

Potentially, a squash merge might be a good idea.

@@ -27,7 +27,7 @@ services:
CONNECT_PLUGIN_PATH: "/usr/share/java,/usr/share/confluent-hub-components"
CONNECT_LOG4J_ROOT_LOGLEVEL: INFO
# CLASSPATH required due to CC-2422
CLASSPATH: /usr/share/java/monitoring-interceptors/monitoring-interceptors-7.7.1.jar
CLASSPATH: /usr/share/java/monitoring-interceptors/monitoring-interceptors-8.0.0-0.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be 7.8.0 since it's to be merged into 7.8.0-post branch

@@ -2,7 +2,7 @@
services:

schema-registry:
image: confluentinc/cp-schema-registry:7.8.0
image: confluentinc/cp-schema-registry:8.0.x-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

there are a bunch of spots in this PR where 7.8.0 is changed to 8.0.x-latest that should be reverted back to 7.8.0 since this is going into 7.8.0-post. also post branches should also reference publicly available tags


docker compose up -d

# # Start Keycloak and Broker instances
Copy link
Contributor

Choose a reason for hiding this comment

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

if all this needs is docker compose up -d, then remove rather than comment out all of the obsolete parts of this file - it can be confusing to have commented out code left behind

@@ -228,9 +271,13 @@ services:
SCHEMA_REGISTRY_CONFLUENT_METADATA_OAUTHBEARER_LOGIN_CLIENT_SECRET: $SR_CLIENT_SECRET

connect:
image: ${DOCKER_REGISTRY}confluentinc/cp-server-connect:${CONFLUENT_DOCKER_TAG:-latest}
#image: ${DOCKER_REGISTRY}confluentinc/cp-server-connect:${CONFLUENT_DOCKER_TAG:-latest}
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be deleted rather than commented, right?

Copy link
Contributor

@davetroiano davetroiano left a comment

Choose a reason for hiding this comment

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

thanks -- works well but I see just a couple more instances of version 8.0 changes to revert

@@ -73,7 +73,7 @@ services:
CONNECT_VALUE_CONVERTER: io.confluent.connect.avro.AvroConverter
CONNECT_VALUE_CONVERTER_SCHEMA_REGISTRY_URL: http://schema-registry:8081
# CLASSPATH required due to CC-2422
CLASSPATH: /usr/share/java/monitoring-interceptors/monitoring-interceptors-7.8.0.jar
CLASSPATH: /usr/share/java/monitoring-interceptors/monitoring-interceptors-8.0.0-0.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

revert

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the hassle. I will check the whole configuration again and fix the remaining settings.

@@ -7,7 +7,7 @@
<parent>
<groupId>io.confluent</groupId>
<artifactId>rest-utils-parent</artifactId>
<version>7.8.0</version>
<version>8.0.0-0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

revert changes in this file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants