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

✨ 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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bc06ee9
Add Secrets
yanmxa Aug 13, 2024
2c085a9
fix integration
yanmxa Aug 13, 2024
d11b91c
fix the e2e
yanmxa Aug 14, 2024
932593b
rename the transport secret
yanmxa Aug 14, 2024
9e9f1b4
roll back unnessary
yanmxa Aug 14, 2024
2719aaf
reply review
yanmxa Aug 15, 2024
0683073
review
yanmxa Aug 15, 2024
8b90efe
fix integration
yanmxa Aug 15, 2024
fc0b6d9
fix e2e
yanmxa Aug 15, 2024
f3ab8a4
fix sonar
yanmxa Aug 16, 2024
ccebc0d
log the kafka user and topics information
yanmxa Aug 16, 2024
4035268
add kafkauser and topics log
yanmxa Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions operator/pkg/controllers/addon/addon_controller_manifests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@ type ManifestsConfig struct {
ImagePullSecretData string
ImagePullPolicy string
LeafHubID string
KafkaAgentSecretName string
KafkaBootstrapServer string
KafkaBootstrapServers string
TransportType string
KafkaCACert string
KafkaClientCert string
KafkaClientKey string
KafkaClientCertSecret string
KafkaConsumerTopic string
KafkaProducerTopic string
KafkaSpecTopic string
KafkaStatusTopic string
MessageCompressionType string
InstallACMHub bool
Channel string
Expand Down Expand Up @@ -223,13 +227,17 @@ func (a *HohAgentAddon) GetValues(cluster *clusterv1.ManagedCluster,
HoHAgentImage: image,
ImagePullPolicy: string(imagePullPolicy),
LeafHubID: cluster.Name,
KafkaAgentSecretName: constants.GHAgentTransportSecret,
KafkaBootstrapServer: kafkaConnection.BootstrapServer,
KafkaBootstrapServers: base64.StdEncoding.EncodeToString([]byte(kafkaConnection.BootstrapServer)),
KafkaCACert: kafkaConnection.CACert,
KafkaClientCert: kafkaConnection.ClientCert,
KafkaClientKey: kafkaConnection.ClientKey,
KafkaClientCertSecret: certificates.AgentCertificateSecretName(),
KafkaConsumerTopic: clusterTopic.SpecTopic,
KafkaSpecTopic: base64.StdEncoding.EncodeToString([]byte(clusterTopic.SpecTopic)),
KafkaProducerTopic: clusterTopic.StatusTopic,
KafkaStatusTopic: base64.StdEncoding.EncodeToString([]byte(clusterTopic.StatusTopic)),
MessageCompressionType: string(operatorconstants.GzipCompressType),
TransportType: string(transport.Kafka),
LeaseDuration: strconv.Itoa(electionConfig.LeaseDuration),
Expand Down
8 changes: 8 additions & 0 deletions ...ers/addon/manifests/templates/agent/multicluster-global-hub-agent-kafka-certs-secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

namespace: {{ .AddonInstallNamespace }}
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.

labels:
addon.open-cluster-management.io/hosted-manifest-location: none
name: {{.KafkaAgentSecretName}}
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.

"spec_topic": {{.KafkaSpecTopic}}
"ca.crt": "{{.KafkaCACert}}"
"client.crt": "{{.KafkaClientCert}}"
"client.key": "{{.KafkaClientKey}}"
Expand Down
1 change: 1 addition & 0 deletions .../addon/manifests/templates/agent/multicluster-global-hub-agent-kafka-cluster-ca-cert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ metadata:
namespace: {{ .AddonInstallNamespace }}
labels:
addon.open-cluster-management.io/hosted-manifest-location: none
name: {{.KafkaAgentSecretName}}
type: Opaque
data:
"ca.crt": "{{.KafkaCACert}}"
Expand Down
16 changes: 13 additions & 3 deletions operator/pkg/controllers/hubofhubs/manager/manager_reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,18 @@ func (r *ManagerReconciler) Reconcile(ctx context.Context,
DatabaseURL: base64.StdEncoding.EncodeToString(
[]byte(storageConn.SuperuserDatabaseURI)),
PostgresCACert: base64.StdEncoding.EncodeToString(storageConn.CACert),
ManagerTransportSecret: constants.GHManagerTransportSecret,
KafkaClusterIdentity: transportConn.Identity,
KafkaCACert: transportConn.CACert,
KafkaClientCert: transportConn.ClientCert,
KafkaClientKey: transportConn.ClientKey,
KafkaClusterID: base64.StdEncoding.EncodeToString([]byte(transportConn.Identity)),
KafkaBootstrapServer: transportConn.BootstrapServer,
KafkaBootstrapServers: base64.StdEncoding.EncodeToString([]byte(transportConn.BootstrapServer)),
KafkaConsumerTopic: config.ManagerStatusTopic(),
KafkaStatusTopic: base64.StdEncoding.EncodeToString([]byte(config.ManagerStatusTopic())),
KafkaProducerTopic: config.GetSpecTopic(),
KafkaSpecTopic: base64.StdEncoding.EncodeToString([]byte(config.GetSpecTopic())),
KafkaCACert: transportConn.CACert,
KafkaClientCert: transportConn.ClientCert,
KafkaClientKey: transportConn.ClientKey,
Namespace: mgh.Namespace,
MessageCompressionType: string(operatorconstants.GzipCompressType),
TransportType: string(transport.Kafka),
Expand Down Expand Up @@ -193,13 +198,18 @@ type ManagerVariables struct {
ProxySessionSecret string
DatabaseURL string
PostgresCACert string
ManagerTransportSecret string
KafkaClusterIdentity string
KafkaClusterID string
KafkaCACert string
KafkaConsumerTopic string
KafkaProducerTopic string
KafkaStatusTopic string
KafkaSpecTopic string
KafkaClientCert string
KafkaClientKey string
KafkaBootstrapServer string
KafkaBootstrapServers string
MessageCompressionType string
TransportType string
Namespace string
Expand Down
16 changes: 16 additions & 0 deletions operator/pkg/controllers/hubofhubs/manager/manifests/manager-transport-secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: v1
kind: Secret
metadata:
name: {{.ManagerTransportSecret}}
namespace: {{.Namespace}}
labels:
name: multicluster-global-hub-manager
type: Opaque
data:
"cluster_id": {{.KafkaClusterID}}
"bootstrap_server": {{.KafkaBootstrapServers}}
"status_topic": {{.KafkaStatusTopic}}
"spec_topic": {{.KafkaSpecTopic}}
"ca.crt": {{.KafkaCACert}}
"client.crt": {{.KafkaClientCert}}
"client.key": {{.KafkaClientKey}}
13 changes: 7 additions & 6 deletions operator/pkg/controllers/hubofhubs/transporter/protocol/strimzi_transporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,19 +419,19 @@ func (k *strimziTransporter) GetConnCredential(clusterName string) (*transport.C
return nil, err
}

userName := config.GetKafkaUserName(clusterName)
if !k.enableTLS {
k.log.Info("the kafka cluster hasn't enable tls for user", "username", userName)
return credential, nil
}

// don't need to load the client cert/key from the kafka user, since it use the external kafkaUser
// userName := config.GetKafkaUserName(clusterName)
// if !k.enableTLS {
// k.log.Info("the kafka cluster hasn't enable tls for user", "username", userName)
// return credential, nil
// }
// if err := k.loadUserCredentail(userName, credential); err != nil {
// return nil, err
// }
return credential, nil
}

// loadUserCredentail add credential with client cert, and key
func (k *strimziTransporter) loadUserCredentail(kafkaUserName string, credential *transport.ConnCredential) error {
kafkaUserSecret := &corev1.Secret{}
err := k.runtimeClient.Get(k.ctx, types.NamespacedName{
Expand All @@ -446,6 +446,7 @@ func (k *strimziTransporter) loadUserCredentail(kafkaUserName string, credential
return nil
}

// getConnCredentailByCluster gets credential with clusterId, bootstrapServer, and serverCA
func (k *strimziTransporter) getConnCredentailByCluster() (*transport.ConnCredential, error) {
kafkaCluster := &kafkav1beta2.Kafka{}
err := k.runtimeClient.Get(k.ctx, types.NamespacedName{
Expand Down
6 changes: 6 additions & 0 deletions pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

GHAgentTransportSecret = "multicluster-global-hub-agent-transport" // #nosec G101
)

// global hub console secret/configmap names
const (
CustomAlertName = "multicluster-global-hub-custom-alerting"
Expand Down
3 changes: 2 additions & 1 deletion pkg/transport/type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ type ClusterTopic struct {
StatusTopic string
}

// ConnCredential is used to connect the transporter instance
// ConnCredential is used to connect the transporter instance. The field is persisted to secret
// need to be encode with base64.StdEncoding.EncodeToString
type ConnCredential struct {
Identity string
BootstrapServer string
Expand Down
4 changes: 3 additions & 1 deletion test/integration/operator/hubofhubs/transporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,9 @@ var _ = Describe("transporter", Ordered, func() {

// topic: update
_, err = trans.EnsureTopic(clusterName)
Expect(err).To(Succeed())
if !errors.IsAlreadyExists(err) {
Expect(err).To(Succeed())
}

err = trans.Prune(clusterName)
Expect(err).To(Succeed())
Expand Down
Loading