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.

Sign up for GitHub

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

Jump to bottom

✨ Add transport secret for global hub manager and agent #1054

Merged
merged 12 commits into from
Aug 19, 2024
Merged

✨ Add transport secret for global hub manager and agent #1054

merged 12 commits into from
Aug 19, 2024

Conversation

yanmxa
Copy link
Member

@yanmxa yanmxa commented Aug 13, 2024 •
edited
Loading

Summary

The transport-config secret for both the manager and agent will look like this:

apiVersion: v1
data:
  kafka.yaml: ...
kind: Secret
metadata:
  creationTimestamp: "2024-08-15T13:18:49Z"
  labels:
    global-hub.open-cluster-management.io/managed-by: global-hub-operator
    name: multicluster-global-hub-manager
  name: transport-config
  namespace: multicluster-global-hub
  ...
type: Opaque

kafka.yaml for manager:

bootstrap.server: 172.18.0.4:30095
topic.status: ^gh-event.*
topic.spec: gh-spec
cluster.id: ZP_ORdm-SBmK17lAFDUtQA
ca.key: ...
client.crt: ...
client.key: ...

kafka.yaml for the Agent

  • BYO | Standalone:
bootstrap.server: 172.18.0.4:30095
topic.status: ^gh-event.*
topic.spec: gh-spec
cluster.id: ZP_ORdm-SBmK17lAFDUtQA
ca.key: ...
client.crt: ...
client.key: ...
  • Built-in:
bootstrap.server: 172.18.0.4:30095
topic.status: ^gh-event.*
topic.spec: gh-spec
cluster.id: ZP_ORdm-SBmK17lAFDUtQA
ca.secret: ...
client.secret: ...

Related issue(s)

Fixes # https://issues.redhat.com/browse/ACM-13019

@yanmxa
Add Secrets
bc06ee9 
Signed-off-by: myan <[email protected]>
@yanmxa
Copy link
Member Author

yanmxa commented Aug 14, 2024

/retest

@yanmxa yanmxa mentioned this pull request Aug 14, 2024
Closed
@yanmxa
Copy link
Member Author

yanmxa commented Aug 14, 2024

/retest

@yanmxa
Copy link
Member Author

yanmxa commented Aug 14, 2024

/cc @clyang82 @ldpliu

@openshift-ci openshift-ci bot requested review from clyang82 and ldpliu August 14, 2024 08:09
clyang82
clyang82 reviewed Aug 15, 2024
View reviewed changes
...ollers/addon/manifests/templates/agent/multicluster-global-hub-agent-kafka-certs-secret.yaml Outdated
@@ -4,10 +4,18 @@ kind: Secret
metadata:
name: kafka-certs-secret
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to kafka-config or something like. it is not only for certs anymore

...ollers/addon/manifests/templates/agent/multicluster-global-hub-agent-kafka-certs-secret.yaml Outdated
annotations:
transport-type: {{.TransportType}}
client-cert-secret: {{.KafkaClientCertSecret}}
cluster-ca-secret: kafka-cluster-ca-cert
Copy link
Contributor

Choose a reason for hiding this comment

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

should not hardcode cluster here. it is kafkacluster CR name. maybe pass as a parameter is better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

...ollers/addon/manifests/templates/agent/multicluster-global-hub-agent-kafka-certs-secret.yaml Outdated
type: Opaque
data:
"bootstrap_server": {{.KafkaBootstrapServers}}
"status_topic": {{.KafkaStatusTopic}}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have a same format? maybe bootstrap.server

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

pkg/constants/constants.go Outdated
@@ -63,6 +63,12 @@ const (
PostgresCAConfigMap = "multicluster-global-hub-postgres-ca"
)

// the global hub transport secret for manager and agent
const (
GHManagerTransportSecret = "multicluster-global-hub-manager-transport" // #nosec G101
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use kakfa-config as secret name for both manager and agent.

Copy link
Member Author

Choose a reason for hiding this comment

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

When the global hub agent supports standalone mode, the agent might send messages to different targets. So kafka is just an option, it might be a webhook credential.

yanmxa added 6 commits August 15, 2024 04:17
@yanmxa
reply review
2719aaf 
Signed-off-by: myan <[email protected]>
@yanmxa
review
0683073 
Signed-off-by: myan <[email protected]>
@yanmxa
fix integration
8b90efe 
Signed-off-by: myan <[email protected]>
@yanmxa
fix e2e
fc0b6d9 
Signed-off-by: myan <[email protected]>
@yanmxa
fix sonar
f3ab8a4 
Signed-off-by: myan <[email protected]>
@yanmxa
log the kafka user and topics information
ccebc0d 
Signed-off-by: myan <[email protected]>
clyang82
clyang82 approved these changes Aug 16, 2024
View reviewed changes
Copy link
Contributor

@clyang82 clyang82 left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci openshift-ci bot added the lgtm label Aug 16, 2024
@openshift-ci openshift-ci bot removed the lgtm label Aug 16, 2024
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@ldpliu
Copy link
Contributor

ldpliu commented Aug 19, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Aug 19, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Aug 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clyang82, ldpliu, yanmxa

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:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 6e393b6 into stolostron:main Aug 19, 2024
13 checks passed
@clyang82 clyang82 mentioned this pull request Aug 22, 2024
Merged
@yanmxa yanmxa deleted the br_add_secrets branch September 20, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clyang82 clyang82 clyang82 approved these changes

@ldpliu ldpliu Awaiting requested review from ldpliu

Assignees

@clyang82 clyang82

@ldpliu ldpliu

Labels
approved dco-signoff: yes lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yanmxa @ldpliu @clyang82