MGMT-14979: add sasl/scraml auth method for kafka notifications #5299

rccrdpccl · 2023-06-16T12:56:22Z

This would enable us to connect to AWS MSK instances, where SASL/PLAIN is not supported.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2023-06-16T12:56:26Z

rccrdpccl · 2023-06-16T12:56:42Z

/hold

codecov · 2023-06-16T13:19:46Z

Codecov Report

Merging #5299 (2eeee27) into master (db09755) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5299      +/-   ##
==========================================
+ Coverage   67.53%   67.54%   +0.01%     
==========================================
  Files         221      221              
  Lines       33108    33156      +48     
==========================================
+ Hits        22358    22395      +37     
- Misses       8735     8748      +13     
+ Partials     2015     2013       -2

Impacted Files	Coverage Δ
pkg/kafka/json_writer.go	`16.32% <0.00%> (-6.54%)`	⬇️

... and 7 files with indirect coverage changes

rccrdpccl · 2023-06-19T12:13:00Z

/cc @gamli75

rccrdpccl · 2023-06-19T12:13:16Z

Tested in integration

gamli75 · 2023-06-19T20:40:52Z

@rccrdpccl do you think we should expose it in openshift/template.yaml?

rccrdpccl · 2023-06-20T08:44:28Z

/cc @jhernand could you please take a look? 🙏

jhernand

Looks good to me, just one very minor suggestion.

jhernand · 2023-06-20T09:05:56Z

pkg/kafka/json_writer.go

-		}
+	mechanism, err := getMechanism(config)
+	if err != nil {
+		return writer, err


I'd say that this should return nil, err. In general if a function returns an error it shouldn't also return the partially created result.

Signed-off-by: Riccardo piccoli <rpiccoli@redhat.com>

jhernand

Looks good, thanks @rccrdpccl.

rccrdpccl · 2023-06-20T14:57:39Z

/retest flake

openshift-ci · 2023-06-20T14:57:43Z

@rccrdpccl: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test e2e-agent-compact-ipv4
/test edge-assisted-operator-catalog-publish-verify
/test edge-ci-index
/test edge-e2e-ai-operator-ztp
/test edge-e2e-ai-operator-ztp-sno-day2-workers
/test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
/test edge-e2e-metal-assisted
/test edge-e2e-metal-assisted-4-10
/test edge-e2e-metal-assisted-4-11
/test edge-e2e-metal-assisted-4-12
/test edge-e2e-metal-assisted-4-9
/test edge-e2e-metal-assisted-cnv
/test edge-e2e-metal-assisted-lvm
/test edge-e2e-metal-assisted-odf
/test edge-images
/test edge-lint
/test edge-subsystem-aws
/test edge-subsystem-kubeapi-aws
/test edge-unit-test
/test edge-verify-generated-code
/test images
/test mce-images

The following commands are available to trigger optional jobs:

/test e2e-agent-ha-dualstack
/test e2e-agent-sno-ipv6
/test edge-e2e-ai-operator-ztp-3masters
/test edge-e2e-ai-operator-ztp-capi
/test edge-e2e-ai-operator-ztp-compact-day2-masters
/test edge-e2e-ai-operator-ztp-compact-day2-workers
/test edge-e2e-ai-operator-ztp-disconnected
/test edge-e2e-ai-operator-ztp-hypershift-zero-nodes
/test edge-e2e-ai-operator-ztp-ipv4v6-3masters
/test edge-e2e-ai-operator-ztp-ipv4v6-3masters-ocp-411
/test edge-e2e-ai-operator-ztp-ipv4v6-sno
/test edge-e2e-ai-operator-ztp-ipv4v6-sno-ocp-411
/test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp-411
/test edge-e2e-ai-operator-ztp-multiarch-sno-ocp-411
/test edge-e2e-ai-operator-ztp-node-labels
/test edge-e2e-ai-operator-ztp-sno-day2-masters
/test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
/test edge-e2e-metal-assisted-4-13
/test edge-e2e-metal-assisted-4-14
/test edge-e2e-metal-assisted-day2
/test edge-e2e-metal-assisted-day2-arm-workers
/test edge-e2e-metal-assisted-day2-single-node
/test edge-e2e-metal-assisted-ipv4v6
/test edge-e2e-metal-assisted-ipv6
/test edge-e2e-metal-assisted-kube-api-late-binding-single-node
/test edge-e2e-metal-assisted-kube-api-late-unbinding-ipv4-single-node
/test edge-e2e-metal-assisted-kube-api-net-suite
/test edge-e2e-metal-assisted-mce
/test edge-e2e-metal-assisted-mce-4-10
/test edge-e2e-metal-assisted-mce-sno
/test edge-e2e-metal-assisted-metallb
/test edge-e2e-metal-assisted-none
/test edge-e2e-metal-assisted-onprem
/test edge-e2e-metal-assisted-single-node
/test edge-e2e-metal-assisted-static-ip-suite
/test edge-e2e-metal-assisted-tang
/test edge-e2e-metal-assisted-tpmv2
/test edge-e2e-metal-assisted-upgrade-agent
/test edge-e2e-nutanix-assisted
/test edge-e2e-nutanix-assisted-2workers
/test edge-e2e-vsphere-assisted
/test edge-e2e-vsphere-assisted-4-12
/test edge-e2e-vsphere-assisted-4-13
/test edge-e2e-vsphere-assisted-umn
/test edge-push-pr-image
/test push-pr-image

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
pull-ci-openshift-assisted-service-master-edge-ci-index
pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted
pull-ci-openshift-assisted-service-master-edge-images
pull-ci-openshift-assisted-service-master-edge-lint
pull-ci-openshift-assisted-service-master-edge-subsystem-aws
pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
pull-ci-openshift-assisted-service-master-edge-unit-test
pull-ci-openshift-assisted-service-master-edge-verify-generated-code
pull-ci-openshift-assisted-service-master-images
pull-ci-openshift-assisted-service-master-mce-images

In response to this:

/retest flake

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rccrdpccl · 2023-06-20T15:03:05Z

/retest
it's just a flake

rccrdpccl · 2023-06-20T15:22:03Z

/retest

gamli75 · 2023-06-20T19:14:09Z

/test edge-e2e-ai-operator-ztp

openshift-ci · 2023-06-21T06:32:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gamli75, jhernand, rccrdpccl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [gamli75,jhernand,rccrdpccl]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gamli75 · 2023-06-21T06:33:34Z

/hold cancel

gamli75 · 2023-06-21T07:04:42Z

/retest

rccrdpccl · 2023-06-21T15:43:44Z

/retest

openshift-ci · 2023-06-21T20:14:51Z

@rccrdpccl: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

…shift#5299) Signed-off-by: Riccardo piccoli <rpiccoli@redhat.com>

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 16, 2023

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 16, 2023

openshift-ci bot requested review from eranco74 and osherdp June 16, 2023 12:56

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 16, 2023

rccrdpccl force-pushed the enable-sasl-scram-for-notifications branch 3 times, most recently from de230e1 to f59ee51 Compare June 16, 2023 16:04

openshift-ci bot requested a review from gamli75 June 19, 2023 12:13

rccrdpccl force-pushed the enable-sasl-scram-for-notifications branch from f59ee51 to bcae8cc Compare June 20, 2023 06:20

jhernand approved these changes Jun 20, 2023

View reviewed changes

openshift-ci bot assigned jhernand Jun 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2023

MGMT-14979: add sasl/scraml auth method for kafka notifications

2eeee27

Signed-off-by: Riccardo piccoli <rpiccoli@redhat.com>

rccrdpccl force-pushed the enable-sasl-scram-for-notifications branch from bcae8cc to 2eeee27 Compare June 20, 2023 09:13

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2023

jhernand approved these changes Jun 20, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2023

gamli75 approved these changes Jun 21, 2023

View reviewed changes

openshift-ci bot assigned gamli75 Jun 21, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 21, 2023

openshift-merge-robot merged commit beb5397 into openshift:master Jun 21, 2023
14 checks passed

danielerez pushed a commit to danielerez/assisted-service that referenced this pull request Oct 15, 2023

MGMT-14979: add sasl/scraml auth method for kafka notifications (open…

bc597ec

…shift#5299) Signed-off-by: Riccardo piccoli <rpiccoli@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-14979: add sasl/scraml auth method for kafka notifications #5299

MGMT-14979: add sasl/scraml auth method for kafka notifications #5299

rccrdpccl commented Jun 16, 2023

openshift-ci-robot commented Jun 16, 2023 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

rccrdpccl commented Jun 16, 2023

codecov bot commented Jun 16, 2023 •

edited

rccrdpccl commented Jun 19, 2023

rccrdpccl commented Jun 19, 2023

gamli75 commented Jun 19, 2023

rccrdpccl commented Jun 20, 2023

jhernand left a comment

jhernand Jun 20, 2023

jhernand left a comment

rccrdpccl commented Jun 20, 2023

openshift-ci bot commented Jun 20, 2023

rccrdpccl commented Jun 20, 2023

rccrdpccl commented Jun 20, 2023

gamli75 commented Jun 20, 2023

openshift-ci bot commented Jun 21, 2023

gamli75 commented Jun 21, 2023

gamli75 commented Jun 21, 2023

rccrdpccl commented Jun 21, 2023

openshift-ci bot commented Jun 21, 2023

MGMT-14979: add sasl/scraml auth method for kafka notifications #5299

MGMT-14979: add sasl/scraml auth method for kafka notifications #5299

Conversation

rccrdpccl commented Jun 16, 2023

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Jun 16, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

rccrdpccl commented Jun 16, 2023

codecov bot commented Jun 16, 2023 • edited

Codecov Report

rccrdpccl commented Jun 19, 2023

rccrdpccl commented Jun 19, 2023

gamli75 commented Jun 19, 2023

rccrdpccl commented Jun 20, 2023

jhernand left a comment

Choose a reason for hiding this comment

jhernand Jun 20, 2023

Choose a reason for hiding this comment

jhernand left a comment

Choose a reason for hiding this comment

rccrdpccl commented Jun 20, 2023

openshift-ci bot commented Jun 20, 2023

rccrdpccl commented Jun 20, 2023

rccrdpccl commented Jun 20, 2023

gamli75 commented Jun 20, 2023

openshift-ci bot commented Jun 21, 2023

gamli75 commented Jun 21, 2023

gamli75 commented Jun 21, 2023

rccrdpccl commented Jun 21, 2023

openshift-ci bot commented Jun 21, 2023

openshift-ci-robot commented Jun 16, 2023 •

edited by openshift-ci bot

codecov bot commented Jun 16, 2023 •

edited