-
Notifications
You must be signed in to change notification settings - Fork 100
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
SGRC-4405 Update Kafka to 3.7.0 and Confluent to 7.6.0 #854
Conversation
92fc4d7
to
6f38e6a
Compare
@@ -34,10 +35,13 @@ public abstract class ConnectClusterBaseIT { | |||
|
|||
@BeforeAll | |||
public void beforeAll() { | |||
Map<String, String> workerConfig = new HashMap<>(); |
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.
This change is caused by: https://kafka.apache.org/documentation.html#connect_plugindiscovery
Note that the default value according to the docs is hybrid_warn
and hybrid_fail
is set explicitly here https://github.com/apache/kafka/blob/trunk/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectCluster.java#L160
@@ -94,7 +98,7 @@ protected final void waitForConnectorStopped(String connectorName) { | |||
try { | |||
connectCluster | |||
.assertions() | |||
.assertConnectorAndTasksAreStopped(connectorName, "Failed to stop the connector"); | |||
.assertConnectorDoesNotExist(connectorName, "Failed to stop the connector"); |
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.
The previous implementation of assertConnectorAndTasksAreStopped
returned true for any connector state other than RUNNING
and was deleted in favour of assertConnectorIsStopped
which checks exact matching of state STOPPED
.
In our case the connector status after deletion is always NOT_FOUND
.
7.2.*) | ||
DOWNLOAD_URL="https://packages.confluent.io/archive/7.2/confluent-community-$CONFLUENT_VERSION.tar.gz" | ||
7.6.*) | ||
DOWNLOAD_URL="https://packages.confluent.io/archive/7.6/confluent-community-$CONFLUENT_VERSION.tar.gz" |
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 have a strong feeling that we can do better than this :)
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.
Should we add a ticket for tech debt?
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 doubt if we ever have enough space to tackle such low priority issues. I will try to fix it by myself in the next PR rather than create a ticket.
|
||
<dependency> | ||
<groupId>io.confluent</groupId> | ||
<artifactId>kafka-schema-registry-client-encryption</artifactId> |
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.
Lack of this dependency caused TestNativeStringAvrosr to fail on a task start with the following exception:
java.util.ServiceConfigurationError: io.confluent.kafka.schemaregistry.rules.RuleExecutor: io.confluent.kafka.schemaregistry.encryption.FieldEncryptionExecutor not a subtype
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.
Same for kafka-schema-rules
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.
LGTM.
Great work.
Overview
SGRC-4405 Existing Vulnerabilities found in Connectors
Automatic security scan detected some vulnerabilities in the connector dependencies:
We can mitigate the majority of vulnerabilities by upgrading Kafka and Confluent. Some of them are still present in the most recent confluent releases:
Pre-review checklist
snowflake.ingestion.method
.Yes
- Added end to end and Unit Tests.No
- it is nearly impossible to protect dependency change with a parameter