From 2c85a30e457f9a375c66c553f9498bb90cbd2c77 Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Wed, 3 Sep 2025 15:15:15 +0800 Subject: [PATCH 01/12] add delete working cert strategy fix go mod redis queue delete contains get & fix cert manager bug add cert manager add nil check for log add double check for cert manager --- api/go.mod | 2 - .../v1alpha1/managers/cert/cert-manager.go | 395 ++++++++++++++++++ .../apis/v1alpha1/managers/managerfactory.go | 3 + .../managers/solution/solution-manager.go | 39 +- api/pkg/apis/v1alpha1/model/deployment.go | 1 + api/pkg/apis/v1alpha1/utils/symphony-api.go | 12 +- .../apis/v1alpha1/vendors/solution-vendor.go | 25 ++ .../apis/v1alpha1/vendors/targets-vendor.go | 114 +---- .../queue/redis/redisQueueProvider.go | 25 +- .../helm/symphony/files/symphony-api.json | 26 ++ 10 files changed, 538 insertions(+), 104 deletions(-) create mode 100644 api/pkg/apis/v1alpha1/managers/cert/cert-manager.go diff --git a/api/go.mod b/api/go.mod index b880bd891..c5e2a1c8c 100644 --- a/api/go.mod +++ b/api/go.mod @@ -16,7 +16,6 @@ require ( k8s.io/api v0.31.3 k8s.io/apimachinery v0.31.3 k8s.io/client-go v0.31.3 - ) require ( @@ -172,7 +171,6 @@ require ( sigs.k8s.io/kustomize/api v0.17.2 // indirect sigs.k8s.io/kustomize/kyaml v0.17.1 // indirect sigs.k8s.io/yaml v1.4.0 - ) require ( diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go new file mode 100644 index 000000000..760a37258 --- /dev/null +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -0,0 +1,395 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + * SPDX-License-Identifier: MIT + */ + +package cert + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/contexts" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/managers" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/secret" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/utils" + "github.com/eclipse-symphony/symphony/coa/pkg/logger" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var ( + cLog = logger.NewLogger("coa.runtime") +) + +type CertManager struct { + managers.Manager + StateProvider states.IStateProvider + SecretProvider secret.ISecretProvider + Config CertManagerConfig +} + +type CertManagerConfig struct { + CAIssuer string + ServiceName string + WorkingCertDuration string + WorkingCertRenewBefore string +} + +func (c *CertManager) Init(context *contexts.VendorContext, config managers.ManagerConfig, providers map[string]providers.IProvider) error { + err := c.Manager.Init(context, config, providers) + if err != nil { + return err + } + + stateProvider, err := managers.GetPersistentStateProvider(config, providers) + if err == nil { + c.StateProvider = stateProvider + } else { + return err + } + + secretProvider, err := managers.GetSecretProvider(config, providers) + if err == nil { + if sp, ok := secretProvider.(secret.ISecretProvider); ok { + c.SecretProvider = sp + } else { + // Try to get from providers map directly + for _, p := range providers { + if sp, ok := p.(secret.ISecretProvider); ok { + c.SecretProvider = sp + break + } + } + if c.SecretProvider == nil { + return v1alpha2.NewCOAError(nil, "secret provider not found", v1alpha2.MissingConfig) + } + } + } else { + return err + } + + // Initialize config with defaults + c.Config = CertManagerConfig{ + CAIssuer: getConfigValue(config, "caIssuer", "symphony-issuer"), + ServiceName: getConfigValue(config, "serviceName", "symphony-service"), + WorkingCertDuration: getConfigValue(config, "workingCertDuration", "2160h"), // 90 days + WorkingCertRenewBefore: getConfigValue(config, "workingCertRenewBefore", "360h"), // 15 days + } + + return nil +} + +func getConfigValue(config managers.ManagerConfig, key, defaultValue string) string { + if val, exists := config.Properties[key]; exists && val != "" { + return val + } + return defaultValue +} + +// CreateWorkingCert creates a working certificate for the specified target +func (c *CertManager) CreateWorkingCert(ctx context.Context, targetName, namespace string) error { + cLog.InfofCtx(ctx, "Creating working cert for target %s in namespace %s", targetName, namespace) + + subject := fmt.Sprintf("CN=%s-%s.%s", namespace, targetName, c.Config.ServiceName) + secretName := fmt.Sprintf("%s-tls", targetName) + + // Create a new GroupVersionKind for the certificate + gvk := schema.GroupVersionKind{ + Group: "cert-manager.io", + Version: "v1", + Kind: "Certificate", + } + + // Create a new unstructured object for the certificate + cert := &unstructured.Unstructured{} + cert.SetGroupVersionKind(gvk) + cert.SetName(targetName) + cert.SetNamespace(namespace) + + spec := map[string]interface{}{ + "secretName": secretName, + "duration": c.Config.WorkingCertDuration, + "renewBefore": c.Config.WorkingCertRenewBefore, + "commonName": subject, + "dnsNames": []string{ + subject, + }, + "issuerRef": map[string]interface{}{ + "name": c.Config.CAIssuer, + "kind": "Issuer", + }, + "subject": map[string]interface{}{ + "organizations": []interface{}{ + c.Config.ServiceName, + }, + }, + "privateKey": map[string]interface{}{ + "algorithm": "RSA", + "size": 2048, + }, + } + + cert.Object["spec"] = spec + + upsertRequest := states.UpsertRequest{ + Value: states.StateEntry{ + ID: targetName, + Body: cert.Object, + }, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": gvk.Group, + "version": gvk.Version, + "resource": "certificates", + "kind": gvk.Kind, + }, + } + + // Check if Certificate already exists + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": gvk.Group, + "version": gvk.Version, + "resource": "certificates", + "kind": gvk.Kind, + }, + } + + _, err := c.StateProvider.Get(ctx, getRequest) + if err == nil { + cLog.InfofCtx(ctx, "Certificate %s already exists, skipping creation", targetName) + return nil + } + + // Certificate doesn't exist, create it + jsonData, _ := json.Marshal(upsertRequest) + cLog.InfofCtx(ctx, "Creating certificate object - %s", jsonData) + _, err = c.StateProvider.Upsert(ctx, upsertRequest) + if err != nil { + cLog.ErrorfCtx(ctx, "Failed to create certificate: %s", err.Error()) + return err + } + + cLog.InfofCtx(ctx, "Successfully created working cert for target %s", targetName) + return nil +} + +// DeleteWorkingCert deletes the working certificate for the specified target +func (c *CertManager) DeleteWorkingCert(ctx context.Context, targetName, namespace string) error { + cLog.InfofCtx(ctx, "Deleting working cert for target %s in namespace %s", targetName, namespace) + + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + // first check if exists + _, err := c.StateProvider.Get(ctx, getRequest) + if err != nil { + cLog.ErrorfCtx(ctx, "Working cert %s not found, cannot delete: %s", targetName, err.Error()) + return err + } + + // if found, then delete + deleteRequest := states.DeleteRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + err = c.StateProvider.Delete(ctx, deleteRequest) + if err != nil && !v1alpha2.IsNotFound(err) { + cLog.ErrorfCtx(ctx, "Failed to delete certificate: %s", err.Error()) + return err + } + + // double check deletion + _, err = c.StateProvider.Get(ctx, getRequest) + if v1alpha2.IsNotFound(err) { + cLog.InfofCtx(ctx, "Successfully deleted working cert for target %s", targetName) + return nil + } + + cLog.ErrorfCtx(ctx, "Certificate %s still exists after delete", targetName) + if err != nil { + return err + } + return fmt.Errorf("certificate %s still exists after delete", targetName) +} + +// GetWorkingCert retrieves the working certificate for the specified target (read-only) +func (c *CertManager) GetWorkingCert(ctx context.Context, targetName, namespace string) (string, string, error) { + cLog.InfofCtx(ctx, "Getting working cert for target %s in namespace %s", targetName, namespace) + + secretName := fmt.Sprintf("%s-tls", targetName) + evalCtx := utils.EvaluationContext{Namespace: namespace} + + // Check if certificate exists first + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + _, err := c.StateProvider.Get(ctx, getRequest) + if err != nil { + return "", "", fmt.Errorf("working certificate not found for target %s: %w", targetName, err) + } + + // Read the certificate and private key from the secret + public, err := c.SecretProvider.Read(ctx, secretName, "tls.crt", evalCtx) + if err != nil { + return "", "", fmt.Errorf("failed to read public certificate: %w", err) + } + + private, err := c.SecretProvider.Read(ctx, secretName, "tls.key", evalCtx) + if err != nil { + return "", "", fmt.Errorf("failed to read private key: %w", err) + } + + // Format certificates (remove newlines) + public = strings.ReplaceAll(public, "\n", " ") + private = strings.ReplaceAll(private, "\n", " ") + + cLog.InfofCtx(ctx, "Successfully retrieved working cert for target %s", targetName) + return public, private, nil +} + +// CheckCertificateReady checks if the certificate is ready and the secret is available +func (c *CertManager) CheckCertificateReady(ctx context.Context, targetName, namespace string) (bool, error) { + // Check Certificate status + ready, err := c.checkCertificateStatus(ctx, targetName, namespace) + if err != nil { + return false, err + } + + if !ready { + return false, nil + } + + // Check if secret exists and has correct type + secretName := fmt.Sprintf("%s-tls", targetName) + secretReady, err := c.checkSecretReady(ctx, secretName, namespace) + if err != nil { + return false, err + } + + return secretReady, nil +} + +// checkCertificateStatus checks if Certificate is ready +func (c *CertManager) checkCertificateStatus(ctx context.Context, certName, namespace string) (bool, error) { + getRequest := states.GetRequest{ + ID: certName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + entry, err := c.StateProvider.Get(ctx, getRequest) + if err != nil { + return false, fmt.Errorf("failed to get certificate: %s", err.Error()) + } + + // Check Certificate status conditions + if status, found := entry.Body.(map[string]interface{})["status"]; found { + if statusMap, ok := status.(map[string]interface{}); ok { + if conditions, found := statusMap["conditions"]; found { + if conditionsArray, ok := conditions.([]interface{}); ok { + for _, condition := range conditionsArray { + if condMap, ok := condition.(map[string]interface{}); ok { + if condType, found := condMap["type"]; found && strings.ToLower(condType.(string)) == "ready" { + if condStatus, found := condMap["status"]; found && strings.ToLower(condStatus.(string)) == "true" { + return true, nil + } + } + } + } + } + } + } + } + + return false, nil +} + +// checkSecretReady checks if secret exists and has the correct type and content +func (c *CertManager) checkSecretReady(ctx context.Context, secretName, namespace string) (bool, error) { + evalCtx := utils.EvaluationContext{Namespace: namespace} + + // Try to read both tls.crt and tls.key to verify secret is complete + _, err := c.SecretProvider.Read(ctx, secretName, "tls.crt", evalCtx) + if err != nil { + return false, nil // Secret not ready yet + } + + _, err = c.SecretProvider.Read(ctx, secretName, "tls.key", evalCtx) + if err != nil { + return false, nil // Secret not complete yet + } + + return true, nil +} + +// WaitForCertificateReady waits for Certificate to be ready and secret to have the correct type and content +func (c *CertManager) WaitForCertificateReady(ctx context.Context, targetName, namespace string) error { + cLog.InfofCtx(ctx, "Waiting for certificate %s to be ready in namespace %s", targetName, namespace) + + // Create a context with timeout for the whole operation + timeoutCtx, cancel := context.WithTimeout(ctx, 120*time.Second) + defer cancel() + + // Simple retry loop instead of using backoff package + ticker := time.NewTicker(2 * time.Second) + defer ticker.Stop() + + for { + select { + case <-timeoutCtx.Done(): + return fmt.Errorf("timeout waiting for certificate %s to be ready", targetName) + case <-ticker.C: + ready, err := c.CheckCertificateReady(timeoutCtx, targetName, namespace) + if err != nil { + cLog.ErrorfCtx(timeoutCtx, "Error checking certificate status: %v", err) + continue + } + + if ready { + cLog.InfofCtx(timeoutCtx, "Certificate %s is ready", targetName) + return nil + } + + cLog.InfofCtx(timeoutCtx, "Certificate %s not ready yet, waiting...", targetName) + } + } +} diff --git a/api/pkg/apis/v1alpha1/managers/managerfactory.go b/api/pkg/apis/v1alpha1/managers/managerfactory.go index deada38a2..ecc1743d2 100644 --- a/api/pkg/apis/v1alpha1/managers/managerfactory.go +++ b/api/pkg/apis/v1alpha1/managers/managerfactory.go @@ -12,6 +12,7 @@ import ( "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/campaigns" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/catalogcontainers" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/catalogs" + "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/cert" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/configs" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/devices" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/instances" @@ -106,6 +107,8 @@ func (c *SymphonyManagerFactory) CreateManager(config cm.ManagerConfig) (cm.IMan manager = &skills.SkillsManager{} case "managers.symphony.trails": manager = &trails.TrailsManager{} + case "managers.symphony.cert": + manager = &cert.CertManager{} } if manager != nil && config.Properties["singleton"] == "true" { c.SingletonsCache[config.Type] = manager diff --git a/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go b/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go index c9c53cc0d..b43e401a2 100644 --- a/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go +++ b/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go @@ -299,6 +299,36 @@ func (s *SolutionManager) ensureRemoteTargetSubscriptions(ctx context.Context, d } } +// cleanupRemoteTargetResourcesAfterDeletion cleans up MQTT subscriptions and Redis queues for deleted remote targets after successful deletion +func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context.Context, targetName string, namespace string) { + mqttBinding := s.VendorContext.GetMQTTBinding() + if mqttBinding != nil { + topic := fmt.Sprintf("symphony/request/%s", targetName) + log.InfofCtx(ctx, " M (Solution): cleaning up MQTT subscription for deleted remote target %s, topic %s", targetName, topic) + + // Unsubscribe from MQTT topic using the dedicated method + if mqttBinding != nil { + if err := mqttBinding.UnsubscribeTopic(topic); err != nil { + log.WarnfCtx(ctx, " M (Solution): failed to unsubscribe from MQTT topic %s for deleted target %s: %s", topic, targetName, err.Error()) + } else { + log.InfofCtx(ctx, " M (Solution): successfully unsubscribed from MQTT topic %s for deleted target %s", topic, targetName) + } + } + + // Clean up Redis queue + if s.QueueProvider != nil { + queueName := fmt.Sprintf("%s-%s", targetName, namespace) + if queueErr := s.QueueProvider.DeleteQueue(ctx, queueName); queueErr != nil { + log.WarnfCtx(ctx, " M (Solution): failed to delete Redis queue %s for deleted target %s: %s", queueName, targetName, queueErr.Error()) + } else { + log.InfofCtx(ctx, " M (Solution): successfully deleted Redis queue %s for deleted target %s", queueName, targetName) + } + } else { + log.WarnfCtx(ctx, " M (Solution): Queue provider not available, skipping queue cleanup for deleted target %s", targetName) + } + } +} + func (s *SolutionManager) getPreviousState(ctx context.Context, instance string, namespace string) *model.SolutionManagerDeploymentState { state, err := s.StateProvider.Get(ctx, states.GetRequest{ ID: instance, @@ -463,7 +493,7 @@ func (s *SolutionManager) handleAllPlanCompletetion(ctx context.Context, summary if !summary.PlanState.Deployment.IsDryRun { if len(summary.PlanState.MergedState.TargetComponent) == 0 && summary.IsRemoval { log.DebugfCtx(ctx, " M (Solution): no assigned components to manage, deleting state") - s.StateProvider.Delete(ctx, states.DeleteRequest{ + err := s.StateProvider.Delete(ctx, states.DeleteRequest{ ID: summary.PlanState.Deployment.Instance.ObjectMeta.Name, Metadata: map[string]interface{}{ "namespace": summary.PlanState.Namespace, @@ -472,6 +502,13 @@ func (s *SolutionManager) handleAllPlanCompletetion(ctx context.Context, summary "resource": DeploymentState, }, }) + if err != nil { + log.ErrorCtx(ctx, " M (Solution): failed to delete state for deployment %+v: %s", summary.PlanState.Deployment, err.Error()) + } + // Only cleanup remote target resources after successful deletion + if err == nil && summary.PlanState.Deployment.RemoteTargetName != "" { + s.cleanupRemoteTargetResourcesAfterDeletion(ctx, summary.PlanState.Deployment.RemoteTargetName, summary.PlanState.Namespace) + } } else { s.StateProvider.Upsert(ctx, states.UpsertRequest{ Value: states.StateEntry{ diff --git a/api/pkg/apis/v1alpha1/model/deployment.go b/api/pkg/apis/v1alpha1/model/deployment.go index d3df4c7e2..d9bff9a4e 100644 --- a/api/pkg/apis/v1alpha1/model/deployment.go +++ b/api/pkg/apis/v1alpha1/model/deployment.go @@ -29,6 +29,7 @@ type DeploymentSpec struct { Hash string `json:"hash,omitempty"` IsDryRun bool `json:"isDryRun,omitempty"` IsInActive bool `json:"isInActive,omitempty"` + RemoteTargetName string `json:"remoteTargetName,omitempty"` } func (d DeploymentSpec) GetComponentSlice() []ComponentSpec { diff --git a/api/pkg/apis/v1alpha1/utils/symphony-api.go b/api/pkg/apis/v1alpha1/utils/symphony-api.go index 6088161ec..7d6c7f6bd 100644 --- a/api/pkg/apis/v1alpha1/utils/symphony-api.go +++ b/api/pkg/apis/v1alpha1/utils/symphony-api.go @@ -663,8 +663,18 @@ func CreateSymphonyDeploymentFromTarget(ctx context.Context, target model.Target scope = constants.DefaultScope } + // Check if this is a remote target by looking for remote-agent components + remoteTargetName := "" + for _, component := range target.Spec.Components { + if component.Type == "remote-agent" { + remoteTargetName = target.ObjectMeta.Name + break + } + } + ret := model.DeploymentSpec{ - ObjectNamespace: namespace, + ObjectNamespace: namespace, + RemoteTargetName: remoteTargetName, } solution := model.SolutionState{ ObjectMeta: model.ObjectMeta{ diff --git a/api/pkg/apis/v1alpha1/vendors/solution-vendor.go b/api/pkg/apis/v1alpha1/vendors/solution-vendor.go index 2393a5296..227b87656 100644 --- a/api/pkg/apis/v1alpha1/vendors/solution-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/solution-vendor.go @@ -14,6 +14,7 @@ import ( "strings" "github.com/eclipse-symphony/symphony/api/constants" + "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/cert" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/solution" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/model" api_utils "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/utils" @@ -32,6 +33,7 @@ import ( type SolutionVendor struct { vendors.Vendor SolutionManager *solution.SolutionManager + CertManager *cert.CertManager } func (o *SolutionVendor) GetInfo() vendors.VendorInfo { @@ -51,10 +53,16 @@ func (e *SolutionVendor) Init(config vendors.VendorConfig, factories []managers. if c, ok := m.(*solution.SolutionManager); ok { e.SolutionManager = c } + if c, ok := m.(*cert.CertManager); ok { + e.CertManager = c + } } if e.SolutionManager == nil { return v1alpha2.NewCOAError(nil, "solution manager is not supplied", v1alpha2.MissingConfig) } + if e.CertManager == nil { + return v1alpha2.NewCOAError(nil, "cert manager is not supplied", v1alpha2.MissingConfig) + } e.Vendor.Context.Subscribe(model.DeploymentStepTopic, v1alpha2.EventHandler{ Handler: func(topic string, event v1alpha2.Event) error { ctx := event.Context @@ -322,6 +330,7 @@ func (c *SolutionVendor) onReconcile(request v1alpha2.COARequest) v1alpha2.COARe targetName = v } } + summary, err := c.SolutionManager.AsyncReconcile(ctx, deployment, remove, namespace, targetName) data, _ := json.Marshal(summary) if err != nil { @@ -331,6 +340,22 @@ func (c *SolutionVendor) onReconcile(request v1alpha2.COARequest) v1alpha2.COARe Body: data, }) } + // Handle certificate management for remote targets only + if deployment.RemoteTargetName != "" { + sLog.InfoCtx(ctx, "V (Solution): managing certificate for remote target: %s", deployment.RemoteTargetName) + if remove { + err = c.CertManager.DeleteWorkingCert(ctx, deployment.RemoteTargetName, namespace) + } else { + err = c.CertManager.CreateWorkingCert(ctx, deployment.RemoteTargetName, namespace) + } + if err != nil { + sLog.ErrorfCtx(ctx, "V (Solution): onReconcile failed POST - certificate management %s", err.Error()) + return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ + State: v1alpha2.InternalError, + Body: []byte(err.Error()), + }) + } + } return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.OK, Body: data, diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go index 9d85b2044..4a724a508 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go @@ -35,8 +35,6 @@ import ( utils2 "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/utils" "github.com/golang-jwt/jwt/v4" "github.com/valyala/fasthttp" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" ) var ( @@ -683,7 +681,7 @@ func (c *TargetsVendor) onHeartBeat(request v1alpha2.COARequest) v1alpha2.COARes return resp } -// getting a certificate for a target +// getting a certificate for a target (read-only) func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COAResponse { ctx, span := observability.StartSpan("Targets Vendor", request.Context, &map[string]string{ "method": "onGetCert", @@ -698,129 +696,51 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo switch request.Method { case fasthttp.MethodPost: - subject := fmt.Sprintf("CN=%s-%s.%s", namespace, id, ServiceName) - // create a new GroupVersionKind for the certificate - gvk := schema.GroupVersionKind{ - Group: "cert-manager.io", - Version: "v1", - Kind: "Certificate", - } - - // create a new unstructured object for the certificate - cert := &unstructured.Unstructured{} - cert.SetGroupVersionKind(gvk) - - cert.SetName(id) - cert.SetNamespace(namespace) - - secretName := fmt.Sprintf("%s-tls", id) - - // Get configurable working certificate duration and renewBefore values with defaults - duration := c.getWorkingCertDuration() - renewBefore := c.getWorkingCertRenewBefore() - - spec := map[string]interface{}{ - "secretName": secretName, - "duration": duration, - "renewBefore": renewBefore, - "commonName": subject, - "dnsNames": []string{ - subject, - }, - "issuerRef": map[string]interface{}{ - "name": CAIssuer, - "kind": "Issuer", - }, - "subject": map[string]interface{}{ - "organizations": []interface{}{ - ServiceName, - }, - }, - "privateKey": map[string]interface{}{ - "algorithm": "RSA", - "size": 2048, - }, - } - - cert.Object["spec"] = spec - - upsertRequest := states.UpsertRequest{ - Value: states.StateEntry{ - ID: id, - Body: cert.Object, - }, - Metadata: map[string]interface{}{ - "namespace": namespace, - "group": gvk.Group, - "version": gvk.Version, - "resource": "certificates", - "kind": gvk.Kind, - }, - } - - // Check if Certificate already exists to avoid concurrent creation + // Check if certificate exists first getRequest := states.GetRequest{ ID: id, Metadata: map[string]interface{}{ "namespace": namespace, - "group": gvk.Group, - "version": gvk.Version, + "group": "cert-manager.io", + "version": "v1", "resource": "certificates", - "kind": gvk.Kind, + "kind": "Certificate", }, } _, err := c.TargetsManager.StateProvider.Get(ctx, getRequest) - if err == nil { - // Certificate already exists, log and proceed to wait - tLog.InfofCtx(ctx, "V (Targets) : Certificate %s already exists, waiting for ready state", id) - } else { - // Certificate doesn't exist, create it - jsonData, _ := json.Marshal(upsertRequest) - tLog.InfofCtx(ctx, "V (Targets) : create certificate object - %s", jsonData) - _, err := c.TargetsManager.StateProvider.Upsert(ctx, upsertRequest) - if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed - %s", err.Error()) - return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.InternalError, - Body: []byte(err.Error()), - }) - } - } - - // Wait for Certificate to be ready and secret to be created with correct type - err = c.waitForCertificateReady(ctx, id, namespace, secretName) if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed waiting for certificate - %s", err.Error()) + tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s: %s", id, err.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.InternalError, - Body: []byte(err.Error()), + State: v1alpha2.NotFound, + Body: []byte(fmt.Sprintf("Working certificate not found for target %s. Please ensure target is properly created and managed through the solution vendor.", id)), }) } - // Use the fixed secret name directly - tLog.InfofCtx(ctx, "V (Targets) : Using fixed secret name: %s", secretName) + secretName := fmt.Sprintf("%s-tls", id) + // Read the certificate and private key from the secret public, err := readSecretWithRetry(ctx, c.TargetsManager.SecretProvider, secretName, "tls.crt", coa_utils.EvaluationContext{Namespace: namespace}) if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed - %s", err.Error()) + tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed to read certificate - %s", err.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.InternalError, - Body: []byte(err.Error()), + State: v1alpha2.NotFound, + Body: []byte(fmt.Sprintf("Failed to read working certificate for target %s. Certificate may not be ready yet.", id)), }) } private, err := readSecretWithRetry(ctx, c.TargetsManager.SecretProvider, secretName, "tls.key", coa_utils.EvaluationContext{Namespace: namespace}) if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed - %s", err.Error()) + tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed to read private key - %s", err.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.InternalError, - Body: []byte(err.Error()), + State: v1alpha2.NotFound, + Body: []byte(fmt.Sprintf("Failed to read working certificate private key for target %s. Certificate may not be ready yet.", id)), }) } public = strings.ReplaceAll(public, "\n", " ") private = strings.ReplaceAll(private, "\n", " ") + tLog.InfofCtx(ctx, "V (Targets) : Successfully retrieved working certificate for target %s", id) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.OK, Body: []byte(fmt.Sprintf("{\"public\":\"%s\",\"private\":\"%s\"}", public, private)), diff --git a/coa/pkg/apis/v1alpha2/providers/queue/redis/redisQueueProvider.go b/coa/pkg/apis/v1alpha2/providers/queue/redis/redisQueueProvider.go index ce34ac3e8..2bee77131 100644 --- a/coa/pkg/apis/v1alpha2/providers/queue/redis/redisQueueProvider.go +++ b/coa/pkg/apis/v1alpha2/providers/queue/redis/redisQueueProvider.go @@ -278,17 +278,36 @@ func (rq *RedisQueueProvider) QueryByPaging(context context.Context, queueName s } func (rq *RedisQueueProvider) DeleteQueue(context context.Context, queue string) error { - // Delete the main stream - err := rq.client.Del(context, queue).Err() + // 1. check if queue exist + exists, err := rq.client.Exists(context, queue).Result() + if err != nil { + return fmt.Errorf("failed to check existence of Redis stream %s: %s", queue, err.Error()) + } + if exists == 0 { + return fmt.Errorf("redis stream %s does not exist", queue) + } + + // 2. delete stream + err = rq.client.Del(context, queue).Err() if err != nil { return fmt.Errorf("failed to delete Redis stream %s: %s", queue, err.Error()) } - // Delete associated metadata (lastID key) + // 3. delete metadata (lastID key) lastIDkey := fmt.Sprintf("%s:lastID", queue) err = rq.client.Del(context, lastIDkey).Err() if err != nil { return fmt.Errorf("failed to delete metadata for Redis stream %s: %s", queue, err.Error()) } + + // 4. check if queue still exists + exists, err = rq.client.Exists(context, queue).Result() + if err != nil { + return fmt.Errorf("failed to re-check existence of Redis stream %s: %s", queue, err.Error()) + } + if exists != 0 { + return fmt.Errorf("redis stream %s still exists after deletion", queue) + } + return nil } diff --git a/packages/helm/symphony/files/symphony-api.json b/packages/helm/symphony/files/symphony-api.json index ac35ad524..497aadd1b 100644 --- a/packages/helm/symphony/files/symphony-api.json +++ b/packages/helm/symphony/files/symphony-api.json @@ -559,6 +559,32 @@ "config": {} } } + }, + { + "name": "cert-manager", + "type": "managers.symphony.cert", + "properties": { + "caIssuer": "{{ .Values.cert.caIssuer | default "symphony-issuer" }}", + "serviceName": "{{ .Values.cert.serviceName | default "symphony-service" }}", + "workingCertDuration": "{{ .Values.cert.workingCertDuration | default "2160h" }}", + "workingCertRenewBefore": "{{ .Values.cert.workingCertRenewBefore | default "360h" }}", + "providers.persistentstate": "k8s-state", + "providers.secret": "k8s-secret" + }, + "providers": { + "k8s-state": { + "type": "providers.state.k8s", + "config": { + "inCluster": true + } + }, + "k8s-secret": { + "type": "providers.secret.k8s", + "config": { + "inCluster": true + } + } + } } ] }, From 7d2a41ad035a70321328a9d5a4ee637cb812f387 Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Mon, 8 Sep 2025 11:10:41 +0800 Subject: [PATCH 02/12] fix cert delete location --- .../apis/v1alpha1/managers/cert/cert-manager.go | 1 - .../managers/solution/solution-manager.go | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go index 760a37258..a2e513f77 100644 --- a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -369,7 +369,6 @@ func (c *CertManager) WaitForCertificateReady(ctx context.Context, targetName, n timeoutCtx, cancel := context.WithTimeout(ctx, 120*time.Second) defer cancel() - // Simple retry loop instead of using backoff package ticker := time.NewTicker(2 * time.Second) defer ticker.Stop() diff --git a/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go b/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go index b43e401a2..7ed6639ac 100644 --- a/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go +++ b/api/pkg/apis/v1alpha1/managers/solution/solution-manager.go @@ -300,7 +300,7 @@ func (s *SolutionManager) ensureRemoteTargetSubscriptions(ctx context.Context, d } // cleanupRemoteTargetResourcesAfterDeletion cleans up MQTT subscriptions and Redis queues for deleted remote targets after successful deletion -func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context.Context, targetName string, namespace string) { +func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context.Context, targetName string, namespace string) error { mqttBinding := s.VendorContext.GetMQTTBinding() if mqttBinding != nil { topic := fmt.Sprintf("symphony/request/%s", targetName) @@ -310,6 +310,7 @@ func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context. if mqttBinding != nil { if err := mqttBinding.UnsubscribeTopic(topic); err != nil { log.WarnfCtx(ctx, " M (Solution): failed to unsubscribe from MQTT topic %s for deleted target %s: %s", topic, targetName, err.Error()) + return err } else { log.InfofCtx(ctx, " M (Solution): successfully unsubscribed from MQTT topic %s for deleted target %s", topic, targetName) } @@ -320,6 +321,7 @@ func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context. queueName := fmt.Sprintf("%s-%s", targetName, namespace) if queueErr := s.QueueProvider.DeleteQueue(ctx, queueName); queueErr != nil { log.WarnfCtx(ctx, " M (Solution): failed to delete Redis queue %s for deleted target %s: %s", queueName, targetName, queueErr.Error()) + return queueErr } else { log.InfofCtx(ctx, " M (Solution): successfully deleted Redis queue %s for deleted target %s", queueName, targetName) } @@ -327,6 +329,7 @@ func (s *SolutionManager) cleanupRemoteTargetResourcesAfterDeletion(ctx context. log.WarnfCtx(ctx, " M (Solution): Queue provider not available, skipping queue cleanup for deleted target %s", targetName) } } + return nil } func (s *SolutionManager) getPreviousState(ctx context.Context, instance string, namespace string) *model.SolutionManagerDeploymentState { @@ -492,6 +495,13 @@ func (s *SolutionManager) handleAllPlanCompletetion(ctx context.Context, summary summary.PlanState.MergedState.ClearAllRemoved() if !summary.PlanState.Deployment.IsDryRun { if len(summary.PlanState.MergedState.TargetComponent) == 0 && summary.IsRemoval { + if summary.PlanState.Deployment.RemoteTargetName != "" { + err := s.cleanupRemoteTargetResourcesAfterDeletion(ctx, summary.PlanState.Deployment.RemoteTargetName, summary.PlanState.Namespace) + if err != nil { + log.ErrorfCtx(ctx, " M (Solution): failed to cleanup remote target resources for %s: %s", summary.PlanState.Deployment.RemoteTargetName, err.Error()) + return err + } + } log.DebugfCtx(ctx, " M (Solution): no assigned components to manage, deleting state") err := s.StateProvider.Delete(ctx, states.DeleteRequest{ ID: summary.PlanState.Deployment.Instance.ObjectMeta.Name, @@ -505,10 +515,7 @@ func (s *SolutionManager) handleAllPlanCompletetion(ctx context.Context, summary if err != nil { log.ErrorCtx(ctx, " M (Solution): failed to delete state for deployment %+v: %s", summary.PlanState.Deployment, err.Error()) } - // Only cleanup remote target resources after successful deletion - if err == nil && summary.PlanState.Deployment.RemoteTargetName != "" { - s.cleanupRemoteTargetResourcesAfterDeletion(ctx, summary.PlanState.Deployment.RemoteTargetName, summary.PlanState.Namespace) - } + } else { s.StateProvider.Upsert(ctx, states.UpsertRequest{ Value: states.StateEntry{ From 97403dcedd329eb991435234543d1b72f5830c29 Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Mon, 8 Sep 2025 13:29:18 +0800 Subject: [PATCH 03/12] add more time retry for get --- api/pkg/apis/v1alpha1/vendors/targets-vendor.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go index 4a724a508..a82633746 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go @@ -696,7 +696,7 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo switch request.Method { case fasthttp.MethodPost: - // Check if certificate exists first + // Check if certificate exists first, with retry: 10s interval, total 120s getRequest := states.GetRequest{ ID: id, Metadata: map[string]interface{}{ @@ -708,9 +708,18 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo }, } - _, err := c.TargetsManager.StateProvider.Get(ctx, getRequest) - if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s: %s", id, err.Error()) + var getErr error + start := time.Now() + for attempt := 0; time.Since(start) < 120*time.Second; attempt++ { + _, getErr = c.TargetsManager.StateProvider.Get(ctx, getRequest) + if getErr == nil { + break + } + tLog.InfofCtx(ctx, "V (Targets) : Working certificate not found for target %s, attempt %d, will retry in 10s: %s", id, attempt+1, getErr.Error()) + time.Sleep(10 * time.Second) + } + if getErr != nil { + tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s after retry: %s", id, getErr.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.NotFound, Body: []byte(fmt.Sprintf("Working certificate not found for target %s. Please ensure target is properly created and managed through the solution vendor.", id)), From cccec0bc26e50e563e5ed528e0841161325178c0 Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Tue, 9 Sep 2025 13:14:25 +0800 Subject: [PATCH 04/12] fix issue --- .../v1alpha1/managers/cert/cert-manager.go | 13 ++++++------- .../apis/v1alpha1/vendors/targets-vendor.go | 16 +++------------- packages/helm/symphony/files/symphony-api.json | 2 -- remote-agent/bootstrap/bootstrap.sh | 18 +++++++++++++++++- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go index a2e513f77..1b8bdcc21 100644 --- a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -10,6 +10,7 @@ import ( "context" "encoding/json" "fmt" + "os" "strings" "time" @@ -26,7 +27,9 @@ import ( ) var ( - cLog = logger.NewLogger("coa.runtime") + cLog = logger.NewLogger("coa.runtime") + CAIssuer = os.Getenv("ISSUER_NAME") + ServiceName = os.Getenv("SYMPHONY_SERVICE_NAME") ) type CertManager struct { @@ -37,8 +40,6 @@ type CertManager struct { } type CertManagerConfig struct { - CAIssuer string - ServiceName string WorkingCertDuration string WorkingCertRenewBefore string } @@ -78,8 +79,6 @@ func (c *CertManager) Init(context *contexts.VendorContext, config managers.Mana // Initialize config with defaults c.Config = CertManagerConfig{ - CAIssuer: getConfigValue(config, "caIssuer", "symphony-issuer"), - ServiceName: getConfigValue(config, "serviceName", "symphony-service"), WorkingCertDuration: getConfigValue(config, "workingCertDuration", "2160h"), // 90 days WorkingCertRenewBefore: getConfigValue(config, "workingCertRenewBefore", "360h"), // 15 days } @@ -98,7 +97,7 @@ func getConfigValue(config managers.ManagerConfig, key, defaultValue string) str func (c *CertManager) CreateWorkingCert(ctx context.Context, targetName, namespace string) error { cLog.InfofCtx(ctx, "Creating working cert for target %s in namespace %s", targetName, namespace) - subject := fmt.Sprintf("CN=%s-%s.%s", namespace, targetName, c.Config.ServiceName) + subject := fmt.Sprintf("CN=%s-%s.%s", namespace, targetName, ServiceName) secretName := fmt.Sprintf("%s-tls", targetName) // Create a new GroupVersionKind for the certificate @@ -123,7 +122,7 @@ func (c *CertManager) CreateWorkingCert(ctx context.Context, targetName, namespa subject, }, "issuerRef": map[string]interface{}{ - "name": c.Config.CAIssuer, + "name": CAIssuer, "kind": "Issuer", }, "subject": map[string]interface{}{ diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go index a82633746..2aff27f7e 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go @@ -696,7 +696,7 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo switch request.Method { case fasthttp.MethodPost: - // Check if certificate exists first, with retry: 10s interval, total 120s + // Check if certificate exists only once (no retry) getRequest := states.GetRequest{ ID: id, Metadata: map[string]interface{}{ @@ -707,19 +707,9 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo "kind": "Certificate", }, } - - var getErr error - start := time.Now() - for attempt := 0; time.Since(start) < 120*time.Second; attempt++ { - _, getErr = c.TargetsManager.StateProvider.Get(ctx, getRequest) - if getErr == nil { - break - } - tLog.InfofCtx(ctx, "V (Targets) : Working certificate not found for target %s, attempt %d, will retry in 10s: %s", id, attempt+1, getErr.Error()) - time.Sleep(10 * time.Second) - } + _, getErr := c.TargetsManager.StateProvider.Get(ctx, getRequest) if getErr != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s after retry: %s", id, getErr.Error()) + tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s: %s", id, getErr.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.NotFound, Body: []byte(fmt.Sprintf("Working certificate not found for target %s. Please ensure target is properly created and managed through the solution vendor.", id)), diff --git a/packages/helm/symphony/files/symphony-api.json b/packages/helm/symphony/files/symphony-api.json index 497aadd1b..326e8c2d6 100644 --- a/packages/helm/symphony/files/symphony-api.json +++ b/packages/helm/symphony/files/symphony-api.json @@ -564,8 +564,6 @@ "name": "cert-manager", "type": "managers.symphony.cert", "properties": { - "caIssuer": "{{ .Values.cert.caIssuer | default "symphony-issuer" }}", - "serviceName": "{{ .Values.cert.serviceName | default "symphony-service" }}", "workingCertDuration": "{{ .Values.cert.workingCertDuration | default "2160h" }}", "workingCertRenewBefore": "{{ .Values.cert.workingCertRenewBefore | default "360h" }}", "providers.persistentstate": "k8s-state", diff --git a/remote-agent/bootstrap/bootstrap.sh b/remote-agent/bootstrap/bootstrap.sh index d069b5790..03b27ee40 100755 --- a/remote-agent/bootstrap/bootstrap.sh +++ b/remote-agent/bootstrap/bootstrap.sh @@ -191,7 +191,23 @@ if [ "$protocol" = "http" ]; then fi # Get certificate - result=$(eval "$curl_cmd -X POST \"$bootstrapCertEndpoint\" -H \"Content-Type: application/json\"") + # Retry logic: try for up to 120s, every 5s + retry_seconds=120 + retry_interval=5 + elapsed=0 + while true; do + result=$(eval "$curl_cmd -X POST \"$bootstrapCertEndpoint\" -H \"Content-Type: application/json\"") + if [ $? -eq 0 ] && [ -n "$result" ]; then + break + fi + if [ $elapsed -ge $retry_seconds ]; then + echo -e "\e[31mError: Failed to call certificate endpoint after $retry_seconds seconds. Please check the endpoint and try again.\e[0m" + exit 1 + fi + echo -e "\e[33mWarning: Certificate endpoint call failed, retrying in $retry_interval seconds... (elapsed: $elapsed s)\e[0m" + sleep $retry_interval + elapsed=$((elapsed+retry_interval)) + done if [ $? -ne 0 ]; then echo -e "\e[31mError: Failed to call certificate endpoint. Please check the endpoint and try again.\e[0m" From 182502e52fdf3d3e6ad6b219614c94199d75167a Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Tue, 9 Sep 2025 15:40:02 +0800 Subject: [PATCH 05/12] replce service name --- api/pkg/apis/v1alpha1/managers/cert/cert-manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go index 1b8bdcc21..965ac15e3 100644 --- a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -127,7 +127,7 @@ func (c *CertManager) CreateWorkingCert(ctx context.Context, targetName, namespa }, "subject": map[string]interface{}{ "organizations": []interface{}{ - c.Config.ServiceName, + ServiceName, }, }, "privateKey": map[string]interface{}{ From bb93566a11bf6e86e3d852a65a470dfb05979234 Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Thu, 11 Sep 2025 13:46:51 +0800 Subject: [PATCH 06/12] target vendor use cert manager to manage cert fix fix function --- .../apis/v1alpha1/vendors/targets-vendor.go | 96 ++++--------------- .../helm/symphony/files/symphony-api.json | 29 +++++- 2 files changed, 43 insertions(+), 82 deletions(-) diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go index 2aff27f7e..468a4ee2d 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go @@ -17,6 +17,7 @@ import ( "github.com/cenkalti/backoff/v4" "github.com/eclipse-symphony/symphony/api/constants" + "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/cert" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/targets" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/model" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/utils" @@ -27,12 +28,10 @@ import ( "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/pubsub" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/secret" - "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states" coa_utils "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/utils" + utils2 "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/utils" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/vendors" "github.com/eclipse-symphony/symphony/coa/pkg/logger" - - utils2 "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/utils" "github.com/golang-jwt/jwt/v4" "github.com/valyala/fasthttp" ) @@ -47,6 +46,7 @@ var ( type TargetsVendor struct { vendors.Vendor TargetsManager *targets.TargetsManager + CertManager *cert.CertManager } func (o *TargetsVendor) GetInfo() vendors.VendorInfo { @@ -696,55 +696,27 @@ func (c *TargetsVendor) onGetCert(request v1alpha2.COARequest) v1alpha2.COARespo switch request.Method { case fasthttp.MethodPost: - // Check if certificate exists only once (no retry) - getRequest := states.GetRequest{ - ID: id, - Metadata: map[string]interface{}{ - "namespace": namespace, - "group": "cert-manager.io", - "version": "v1", - "resource": "certificates", - "kind": "Certificate", - }, - } - _, getErr := c.TargetsManager.StateProvider.Get(ctx, getRequest) - if getErr != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : Working certificate not found for target %s: %s", id, getErr.Error()) - return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.NotFound, - Body: []byte(fmt.Sprintf("Working certificate not found for target %s. Please ensure target is properly created and managed through the solution vendor.", id)), - }) - } - - secretName := fmt.Sprintf("%s-tls", id) - - // Read the certificate and private key from the secret - public, err := readSecretWithRetry(ctx, c.TargetsManager.SecretProvider, secretName, "tls.crt", coa_utils.EvaluationContext{Namespace: namespace}) - if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed to read certificate - %s", err.Error()) + // Use the CertManager to get or create a working certificate for the target + if c.CertManager == nil { + tLog.ErrorfCtx(ctx, "V (Targets) : CertManager not available for onGetCert") return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ - State: v1alpha2.NotFound, - Body: []byte(fmt.Sprintf("Failed to read working certificate for target %s. Certificate may not be ready yet.", id)), + State: v1alpha2.InternalError, + Body: []byte("CertManager not available"), }) } - private, err := readSecretWithRetry(ctx, c.TargetsManager.SecretProvider, secretName, "tls.key", coa_utils.EvaluationContext{Namespace: namespace}) + public, private, err := c.CertManager.GetWorkingCert(ctx, id, namespace) if err != nil { - tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed to read private key - %s", err.Error()) + tLog.ErrorfCtx(ctx, "V (Targets) : onGetCert failed to get working cert - %s", err.Error()) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.NotFound, - Body: []byte(fmt.Sprintf("Failed to read working certificate private key for target %s. Certificate may not be ready yet.", id)), + Body: []byte(fmt.Sprintf("Failed to get working certificate for target %s: %s", id, err.Error())), }) } - - public = strings.ReplaceAll(public, "\n", " ") - private = strings.ReplaceAll(private, "\n", " ") - tLog.InfofCtx(ctx, "V (Targets) : Successfully retrieved working certificate for target %s", id) return observ_utils.CloseSpanWithCOAResponse(span, v1alpha2.COAResponse{ State: v1alpha2.OK, Body: []byte(fmt.Sprintf("{\"public\":\"%s\",\"private\":\"%s\"}", public, private)), }) - } tLog.ErrorCtx(ctx, "V (Targets) : onGetCert failed - method not allowed") resp := v1alpha2.COAResponse{ @@ -765,8 +737,8 @@ func (c *TargetsVendor) waitForCertificateReady(ctx context.Context, certName, n defer cancel() op := func() error { - // Check Certificate status - ready, err := c.checkCertificateStatus(timeoutCtx, certName, namespace) + // Check if Certificate is ready + ready, err := c.checkCertificateReady(timeoutCtx, certName, namespace) if err != nil { tLog.ErrorfCtx(timeoutCtx, "V (Targets) : error checking certificate status: %v", err) return err @@ -809,44 +781,12 @@ func (c *TargetsVendor) waitForCertificateReady(ctx context.Context, certName, n return nil } -// checkCertificateStatus checks if Certificate is ready -func (c *TargetsVendor) checkCertificateStatus(ctx context.Context, certName, namespace string) (bool, error) { - getRequest := states.GetRequest{ - ID: certName, - Metadata: map[string]interface{}{ - "namespace": namespace, - "group": "cert-manager.io", - "version": "v1", - "resource": "certificates", - "kind": "Certificate", - }, +// checks if Certificate is ready +func (c *TargetsVendor) checkCertificateReady(ctx context.Context, certName, namespace string) (bool, error) { + if c.CertManager == nil { + return false, fmt.Errorf("CertManager not available") } - - entry, err := c.TargetsManager.StateProvider.Get(ctx, getRequest) - if err != nil { - return false, fmt.Errorf("failed to get certificate: %s", err.Error()) - } - - // Check Certificate status conditions - if status, found := entry.Body.(map[string]interface{})["status"]; found { - if statusMap, ok := status.(map[string]interface{}); ok { - if conditions, found := statusMap["conditions"]; found { - if conditionsArray, ok := conditions.([]interface{}); ok { - for _, condition := range conditionsArray { - if condMap, ok := condition.(map[string]interface{}); ok { - if condType, found := condMap["type"]; found && strings.EqualFold(condType.(string), "ready") { - if condStatus, found := condMap["status"]; found && strings.EqualFold(condStatus.(string), "true") { - return true, nil - } - } - } - } - } - } - } - } - - return false, nil + return c.CertManager.CheckCertificateReady(ctx, certName, namespace) } // checkSecretReady checks if secret exists and has the correct type and content diff --git a/packages/helm/symphony/files/symphony-api.json b/packages/helm/symphony/files/symphony-api.json index 326e8c2d6..71d121ba6 100644 --- a/packages/helm/symphony/files/symphony-api.json +++ b/packages/helm/symphony/files/symphony-api.json @@ -328,10 +328,7 @@ { "type": "vendors.targets", "route": "targets", - "properties": { - "workingCertDuration": "{{ .Values.targets.workingCertDuration | default "2160h" }}", - "workingCertRenewBefore": "{{ .Values.targets.workingCertRenewBefore | default "360h" }}" - }, + "properties": {}, "managers": [ { "name": "targets-manager", @@ -353,6 +350,30 @@ } } } + }, + { + "name": "cert-manager", + "type": "managers.symphony.cert", + "properties": { + "workingCertDuration": "{{ .Values.cert.workingCertDuration | default "2160h" }}", + "workingCertRenewBefore": "{{ .Values.cert.workingCertRenewBefore | default "360h" }}", + "providers.persistentstate": "k8s-state", + "providers.secret": "k8s-secret" + }, + "providers": { + "k8s-state": { + "type": "providers.state.k8s", + "config": { + "inCluster": true + } + }, + "k8s-secret": { + "type": "providers.secret.k8s", + "config": { + "inCluster": true + } + } + } } ] }, From eacfe2e2c363dbf88540ae001aa90d60be59322c Mon Sep 17 00:00:00 2001 From: Jiaxin Yan Date: Thu, 11 Sep 2025 16:53:59 +0800 Subject: [PATCH 07/12] fix --- .../apis/v1alpha1/vendors/targets-vendor.go | 42 +++---------------- 1 file changed, 6 insertions(+), 36 deletions(-) diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go index 468a4ee2d..c23631b71 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor.go @@ -66,50 +66,20 @@ func (e *TargetsVendor) Init(config vendors.VendorConfig, factories []managers.I if c, ok := m.(*targets.TargetsManager); ok { e.TargetsManager = c } + if c, ok := m.(*cert.CertManager); ok { + e.CertManager = c + } } if e.TargetsManager == nil { return v1alpha2.NewCOAError(nil, "targets manager is not supplied", v1alpha2.MissingConfig) } + if e.CertManager == nil { + return v1alpha2.NewCOAError(nil, "cert manager is not supplied to targets manager", v1alpha2.MissingConfig) + } return nil } -// getWorkingCertDuration returns the configurable working certificate duration with a default of 90 days (2160h) -// This is used for working certificates generated by the /targets/getcert API endpoint for HTTP mode remote agents -// This can be configured in the vendor properties with the key "workingCertDuration" -// Example configuration in symphony-api.json: -// -// { -// "type": "vendors.targets", -// "properties": { -// "workingCertDuration": "4320h" // 180 days -// } -// } -func (c *TargetsVendor) getWorkingCertDuration() string { - if duration, exists := c.Config.Properties["workingCertDuration"]; exists && duration != "" { - return duration - } - return "2160h" // 90 days default -} - -// getWorkingCertRenewBefore returns the configurable working certificate renewBefore with a default of 15 days (360h) -// This is used for working certificates generated by the /targets/getcert API endpoint for HTTP mode remote agents -// This can be configured in the vendor properties with the key "workingCertRenewBefore" -// Example configuration in symphony-api.json: -// -// { -// "type": "vendors.targets", -// "properties": { -// "workingCertRenewBefore": "720h" // 30 days -// } -// } -func (c *TargetsVendor) getWorkingCertRenewBefore() string { - if renewBefore, exists := c.Config.Properties["workingCertRenewBefore"]; exists && renewBefore != "" { - return renewBefore - } - return "360h" // 15 days default -} - func (o *TargetsVendor) GetEndpoints() []v1alpha2.Endpoint { route := "targets" if o.Route != "" { From 77ea32b294f7f6df1300b7dfe8e56ac2aadddc1b Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Thu, 11 Sep 2025 19:44:20 +0800 Subject: [PATCH 08/12] fix ut --- .../v1alpha1/vendors/targets-vendor_test.go | 87 +++++++++++-------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go index 730912ef7..24734898d 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go @@ -12,6 +12,7 @@ import ( "testing" sym_mgr "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers" + "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers/cert" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/model" "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/validation" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2" @@ -19,7 +20,6 @@ import ( "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/pubsub/memory" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/secret/mock" - "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states/memorystate" "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/vendors" "github.com/google/uuid" @@ -27,6 +27,28 @@ import ( "github.com/valyala/fasthttp" ) +// MockCertManager implements a mock certificate manager for testing +type MockCertManager struct{} + +func (m *MockCertManager) GetWorkingCert(ctx context.Context, targetName, namespace string) (string, string, error) { + // Return mock certificate data + public := "-----BEGIN CERTIFICATE----- mock-public-cert-data -----END CERTIFICATE-----" + private := "-----BEGIN PRIVATE KEY----- mock-private-key-data -----END PRIVATE KEY-----" + return public, private, nil +} + +func (m *MockCertManager) CreateWorkingCert(ctx context.Context, targetName, namespace string) error { + return nil +} + +func (m *MockCertManager) DeleteWorkingCert(ctx context.Context, targetName, namespace string) error { + return nil +} + +func (m *MockCertManager) CheckCertificateReady(ctx context.Context, targetName, namespace string) (bool, error) { + return true, nil +} + func TestTargetsEndpoints(t *testing.T) { vendor := createTargetsVendor() vendor.Route = "targets" @@ -80,6 +102,14 @@ func createTargetsVendor() TargetsVendor { vendor.Config.Properties["useJobManager"] = "true" vendor.TargetsManager.TargetValidator = validation.NewTargetValidator(nil, nil) vendor.TargetsManager.SecretProvider = &secretProvider + + // Set up mock CertManager - create a real CertManager but with mock providers + mockCertManager := &cert.CertManager{ + StateProvider: &stateProvider, + SecretProvider: &secretProvider, + } + vendor.CertManager = mockCertManager + return vendor } func TestTargetsOnRegistry(t *testing.T) { @@ -284,6 +314,7 @@ func TestTargetsOnHeartbeats(t *testing.T) { func TestTargetsOnGetCert(t *testing.T) { vendor := createTargetsVendor() + // Register a target first target := model.TargetState{ Spec: &model.TargetSpec{ DisplayName: "target1-v1", @@ -314,54 +345,38 @@ func TestTargetsOnGetCert(t *testing.T) { }) assert.Equal(t, v1alpha2.OK, resp.State) - // Pre-create a mock certificate in ready state to simulate cert-manager behavior - certObj := map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "target1-v1", - "namespace": "default", - }, - "spec": map[string]interface{}{ - "secretName": "target1-v1-tls", - }, - "status": map[string]interface{}{ - "conditions": []interface{}{ - map[string]interface{}{ - "type": "Ready", - "status": "True", - }, - }, - }, - } + ctx := context.Background() + targetName := "target1-v1" + namespace := "default" - // Store the mock certificate in state - upsertRequest := states.UpsertRequest{ - Value: states.StateEntry{ - ID: "target1-v1", - Body: certObj, - }, - Metadata: map[string]interface{}{ - "namespace": "default", - "group": "cert-manager.io", - "version": "v1", - "resource": "certificates", - "kind": "Certificate", - }, - } - vendor.TargetsManager.StateProvider.Upsert(context.Background(), upsertRequest) + // Use CertManager.CreateWorkingCert to create certificate + err := vendor.CertManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + // Test the onGetCert endpoint resp = vendor.onGetCert(v1alpha2.COARequest{ Method: fasthttp.MethodPost, Parameters: map[string]string{ - "__name": "target1-v1", + "__name": targetName, }, - Context: context.Background(), + Context: ctx, }) assert.Equal(t, v1alpha2.OK, resp.State) + // Verify response contains certificate data var certResponse map[string]interface{} json.Unmarshal(resp.Body, &certResponse) assert.Contains(t, certResponse, "public") assert.Contains(t, certResponse, "private") + + // Verify certificate data is not empty and follows MockSecretProvider behavior + // MockSecretProvider returns "secretName>>fieldName" format + public := certResponse["public"].(string) + private := certResponse["private"].(string) + assert.NotEmpty(t, public) + assert.NotEmpty(t, private) + assert.Contains(t, public, "target1-v1-tls>>tls.crt") + assert.Contains(t, private, "target1-v1-tls>>tls.key") } func TestTargetsOnUpdateTopology(t *testing.T) { From 2511637d9d577b5b8fbe1a83722abf37b00593fa Mon Sep 17 00:00:00 2001 From: yanjiaxin534 Date: Thu, 11 Sep 2025 19:49:33 +0800 Subject: [PATCH 09/12] add cert manager ut --- .../managers/cert/cert-manager_test.go | 442 ++++++++++++++++++ 1 file changed, 442 insertions(+) create mode 100644 api/pkg/apis/v1alpha1/managers/cert/cert-manager_test.go diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager_test.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager_test.go new file mode 100644 index 000000000..8254bafb1 --- /dev/null +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager_test.go @@ -0,0 +1,442 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + * SPDX-License-Identifier: MIT + */ + +package cert + +import ( + "context" + "testing" + + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/contexts" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/managers" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/secret/mock" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states" + "github.com/eclipse-symphony/symphony/coa/pkg/apis/v1alpha2/providers/states/memorystate" + "github.com/stretchr/testify/assert" +) + +func createCertManager(t *testing.T) *CertManager { + stateProvider := memorystate.MemoryStateProvider{} + err := stateProvider.Init(memorystate.MemoryStateProviderConfig{}) + assert.NoError(t, err) + + secretProvider := mock.MockSecretProvider{} + err = secretProvider.Init(mock.MockSecretProviderConfig{Name: "test-secret"}) + assert.NoError(t, err) + + certManager := &CertManager{} + + config := managers.ManagerConfig{ + Properties: map[string]string{ + "workingCertDuration": "2160h", + "workingCertRenewBefore": "360h", + "providers.persistentstate": "state", + "providers.secret": "secret", + }, + Providers: map[string]managers.ProviderConfig{ + "state": { + Type: "providers.state.memory", + Config: memorystate.MemoryStateProviderConfig{}, + }, + "secret": { + Type: "providers.secret.mock", + Config: mock.MockSecretProviderConfig{Name: "test-secret"}, + }, + }, + } + + providers := map[string]providers.IProvider{ + "state": &stateProvider, + "secret": &secretProvider, + } + + vendorContext := &contexts.VendorContext{} + err = certManager.Init(vendorContext, config, providers) + assert.NoError(t, err) + + return certManager +} + +func TestCertManagerInit(t *testing.T) { + stateProvider := memorystate.MemoryStateProvider{} + err := stateProvider.Init(memorystate.MemoryStateProviderConfig{}) + assert.NoError(t, err) + + secretProvider := mock.MockSecretProvider{} + err = secretProvider.Init(mock.MockSecretProviderConfig{Name: "test-secret"}) + assert.NoError(t, err) + + certManager := &CertManager{} + + config := managers.ManagerConfig{ + Properties: map[string]string{ + "workingCertDuration": "1440h", + "workingCertRenewBefore": "240h", + "providers.persistentstate": "state", + "providers.secret": "secret", + }, + Providers: map[string]managers.ProviderConfig{ + "state": { + Type: "providers.state.memory", + Config: memorystate.MemoryStateProviderConfig{}, + }, + "secret": { + Type: "providers.secret.mock", + Config: mock.MockSecretProviderConfig{Name: "test-secret"}, + }, + }, + } + + providers := map[string]providers.IProvider{ + "state": &stateProvider, + "secret": &secretProvider, + } + + vendorContext := &contexts.VendorContext{} + err = certManager.Init(vendorContext, config, providers) + assert.NoError(t, err) + + // Verify configuration + assert.Equal(t, "1440h", certManager.Config.WorkingCertDuration) + assert.Equal(t, "240h", certManager.Config.WorkingCertRenewBefore) + assert.NotNil(t, certManager.StateProvider) + assert.NotNil(t, certManager.SecretProvider) +} + +func TestCertManagerInitWithDefaults(t *testing.T) { + stateProvider := memorystate.MemoryStateProvider{} + err := stateProvider.Init(memorystate.MemoryStateProviderConfig{}) + assert.NoError(t, err) + + secretProvider := mock.MockSecretProvider{} + err = secretProvider.Init(mock.MockSecretProviderConfig{Name: "test-secret"}) + assert.NoError(t, err) + + certManager := &CertManager{} + + config := managers.ManagerConfig{ + Properties: map[string]string{ + "providers.persistentstate": "state", + "providers.secret": "secret", + }, + Providers: map[string]managers.ProviderConfig{ + "state": { + Type: "providers.state.memory", + Config: memorystate.MemoryStateProviderConfig{}, + }, + "secret": { + Type: "providers.secret.mock", + Config: mock.MockSecretProviderConfig{Name: "test-secret"}, + }, + }, + } + + providers := map[string]providers.IProvider{ + "state": &stateProvider, + "secret": &secretProvider, + } + + vendorContext := &contexts.VendorContext{} + err = certManager.Init(vendorContext, config, providers) + assert.NoError(t, err) + + // Verify default values + assert.Equal(t, "2160h", certManager.Config.WorkingCertDuration) + assert.Equal(t, "360h", certManager.Config.WorkingCertRenewBefore) +} + +func TestCreateWorkingCert(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "test-target" + namespace := "test-namespace" + + // Test creating a certificate + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Verify certificate was created in StateProvider + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + entry, err := certManager.StateProvider.Get(ctx, getRequest) + assert.NoError(t, err) + assert.NotNil(t, entry.Body) + + // Verify certificate structure + cert := entry.Body.(map[string]interface{}) + assert.Equal(t, targetName, cert["metadata"].(map[string]interface{})["name"]) + assert.Equal(t, namespace, cert["metadata"].(map[string]interface{})["namespace"]) + + spec := cert["spec"].(map[string]interface{}) + assert.Equal(t, "test-target-tls", spec["secretName"]) + assert.Equal(t, "2160h", spec["duration"]) + assert.Equal(t, "360h", spec["renewBefore"]) +} + +func TestCreateWorkingCertAlreadyExists(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "test-target" + namespace := "test-namespace" + + // Create certificate first time + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Create again - should not error + err = certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) +} + +func TestDeleteWorkingCert(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "test-target" + namespace := "test-namespace" + + // Create certificate first + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Delete certificate + err = certManager.DeleteWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Verify certificate was deleted + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + _, err = certManager.StateProvider.Get(ctx, getRequest) + assert.Error(t, err) + assert.True(t, v1alpha2.IsNotFound(err)) +} + +func TestDeleteWorkingCertNotFound(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "non-existent-target" + namespace := "test-namespace" + + // Try to delete non-existent certificate + err := certManager.DeleteWorkingCert(ctx, targetName, namespace) + assert.Error(t, err) +} + +func TestGetWorkingCert(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "test-target" + namespace := "test-namespace" + + // Create certificate first + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Get certificate + public, private, err := certManager.GetWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + assert.NotEmpty(t, public) + assert.NotEmpty(t, private) + + // Verify MockSecretProvider format: "secretName>>fieldName" + expectedSecretName := "test-target-tls" + assert.Equal(t, expectedSecretName+">>tls.crt", public) + assert.Equal(t, expectedSecretName+">>tls.key", private) +} + +func TestGetWorkingCertNotFound(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "non-existent-target" + namespace := "test-namespace" + + // Try to get non-existent certificate + _, _, err := certManager.GetWorkingCert(ctx, targetName, namespace) + assert.Error(t, err) + assert.Contains(t, err.Error(), "working certificate not found") +} + +func TestCheckCertificateReady(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "test-target" + namespace := "test-namespace" + + // Create certificate first + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Certificate without Ready status should return false + ready, err := certManager.CheckCertificateReady(ctx, targetName, namespace) + assert.NoError(t, err) + assert.False(t, ready) + + // Update certificate with Ready status + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + entry, err := certManager.StateProvider.Get(ctx, getRequest) + assert.NoError(t, err) + + cert := entry.Body.(map[string]interface{}) + cert["status"] = map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{ + "type": "Ready", + "status": "True", + }, + }, + } + + upsertRequest := states.UpsertRequest{ + Value: states.StateEntry{ + ID: targetName, + Body: cert, + }, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + _, err = certManager.StateProvider.Upsert(ctx, upsertRequest) + assert.NoError(t, err) + + // Now certificate should be ready + ready, err = certManager.CheckCertificateReady(ctx, targetName, namespace) + assert.NoError(t, err) + assert.True(t, ready) +} + +func TestCheckCertificateReadyNotFound(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "non-existent-target" + namespace := "test-namespace" + + // Check non-existent certificate + _, err := certManager.CheckCertificateReady(ctx, targetName, namespace) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to get certificate") +} + +func TestCheckSecretReady(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + secretName := "test-secret" + namespace := "test-namespace" + + // MockSecretProvider always returns data, so secret should be ready + ready, err := certManager.checkSecretReady(ctx, secretName, namespace) + assert.NoError(t, err) + assert.True(t, ready) +} + +func TestGetConfigValue(t *testing.T) { + config := managers.ManagerConfig{ + Properties: map[string]string{ + "testKey": "testValue", + }, + } + + // Test existing key + value := getConfigValue(config, "testKey", "defaultValue") + assert.Equal(t, "testValue", value) + + // Test non-existent key + value = getConfigValue(config, "nonExistentKey", "defaultValue") + assert.Equal(t, "defaultValue", value) + + // Test empty value + config.Properties["emptyKey"] = "" + value = getConfigValue(config, "emptyKey", "defaultValue") + assert.Equal(t, "defaultValue", value) +} + +func TestCertManagerWithCustomSubject(t *testing.T) { + certManager := createCertManager(t) + ctx := context.Background() + targetName := "custom-target" + namespace := "custom-namespace" + + // Set custom service name for testing + originalServiceName := ServiceName + ServiceName = "test-service" + defer func() { + ServiceName = originalServiceName + }() + + err := certManager.CreateWorkingCert(ctx, targetName, namespace) + assert.NoError(t, err) + + // Verify certificate was created with custom subject + getRequest := states.GetRequest{ + ID: targetName, + Metadata: map[string]interface{}{ + "namespace": namespace, + "group": "cert-manager.io", + "version": "v1", + "resource": "certificates", + "kind": "Certificate", + }, + } + + entry, err := certManager.StateProvider.Get(ctx, getRequest) + assert.NoError(t, err) + + cert := entry.Body.(map[string]interface{}) + spec := cert["spec"].(map[string]interface{}) + expectedCommonName := "CN=custom-namespace-custom-target.test-service" + assert.Equal(t, expectedCommonName, spec["commonName"]) + + dnsNames := spec["dnsNames"].([]interface{}) + assert.Contains(t, dnsNames, expectedCommonName) +} + +func TestCertManagerWithNilProviders(t *testing.T) { + certManager := &CertManager{} + + config := managers.ManagerConfig{ + Properties: map[string]string{}, + } + + // Test with nil providers + providers := map[string]providers.IProvider{} + + vendorContext := &contexts.VendorContext{} + err := certManager.Init(vendorContext, config, providers) + assert.Error(t, err) +} From 18e8c5b7f4e008f15714730e7542b024e3262c1d Mon Sep 17 00:00:00 2001 From: Jiaxin Yan Date: Thu, 11 Sep 2025 21:54:55 +0800 Subject: [PATCH 10/12] check status then return --- .../v1alpha1/managers/cert/cert-manager.go | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go index 965ac15e3..5f9b36b9c 100644 --- a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -244,21 +244,22 @@ func (c *CertManager) GetWorkingCert(ctx context.Context, targetName, namespace secretName := fmt.Sprintf("%s-tls", targetName) evalCtx := utils.EvaluationContext{Namespace: namespace} - // Check if certificate exists first - getRequest := states.GetRequest{ - ID: targetName, - Metadata: map[string]interface{}{ - "namespace": namespace, - "group": "cert-manager.io", - "version": "v1", - "resource": "certificates", - "kind": "Certificate", - }, + // Diagnostic: log provider types to help debug provider/initialization mismatches + cLog.InfofCtx(ctx, "CertManager providers -> StateProvider type: %T, SecretProvider type: %T", c.StateProvider, c.SecretProvider) + + // Ensure providers are initialized + if c.StateProvider == nil { + cLog.ErrorfCtx(ctx, "StateProvider is nil in CertManager") + return "", "", fmt.Errorf("state provider not configured for cert manager") + } + if c.SecretProvider == nil { + cLog.ErrorfCtx(ctx, "SecretProvider is nil in CertManager") + return "", "", fmt.Errorf("secret provider not configured for cert manager") } - _, err := c.StateProvider.Get(ctx, getRequest) - if err != nil { - return "", "", fmt.Errorf("working certificate not found for target %s: %w", targetName, err) + // Wait for certificate + secret to be ready (handles races where the cert object exists shortly after creation) + if err := c.WaitForCertificateReady(ctx, targetName, namespace); err != nil { + return "", "", fmt.Errorf("working certificate not ready for target %s: %w", targetName, err) } // Read the certificate and private key from the secret From e7ed2aeb73304f129391ce42b38396f66cc51091 Mon Sep 17 00:00:00 2001 From: Jiaxin Yan Date: Thu, 11 Sep 2025 23:08:49 +0800 Subject: [PATCH 11/12] add read secret first --- .../apis/v1alpha1/managers/cert/cert-manager.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go index 5f9b36b9c..977bf5746 100644 --- a/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go +++ b/api/pkg/apis/v1alpha1/managers/cert/cert-manager.go @@ -257,12 +257,25 @@ func (c *CertManager) GetWorkingCert(ctx context.Context, targetName, namespace return "", "", fmt.Errorf("secret provider not configured for cert manager") } + // Fast path: try to read the secret immediately (useful for tests and cases where secret already exists) + public, perr := c.SecretProvider.Read(ctx, secretName, "tls.crt", evalCtx) + if perr == nil { + private, perr := c.SecretProvider.Read(ctx, secretName, "tls.key", evalCtx) + if perr == nil { + // Format certificates (remove newlines) + public = strings.ReplaceAll(public, "\n", " ") + private = strings.ReplaceAll(private, "\n", " ") + cLog.InfofCtx(ctx, "Successfully retrieved working cert for target %s (fast path)", targetName) + return public, private, nil + } + } + // Wait for certificate + secret to be ready (handles races where the cert object exists shortly after creation) if err := c.WaitForCertificateReady(ctx, targetName, namespace); err != nil { return "", "", fmt.Errorf("working certificate not ready for target %s: %w", targetName, err) } - // Read the certificate and private key from the secret + // Read the certificate and private key from the secret (after wait) public, err := c.SecretProvider.Read(ctx, secretName, "tls.crt", evalCtx) if err != nil { return "", "", fmt.Errorf("failed to read public certificate: %w", err) From b7e3844d69ab45bd3b0b600de62581b3a184bca5 Mon Sep 17 00:00:00 2001 From: Jiaxin Yan Date: Fri, 12 Sep 2025 06:21:33 +0800 Subject: [PATCH 12/12] use mock --- .../apis/v1alpha1/vendors/targets-vendor_test.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go b/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go index 24734898d..1c020243c 100644 --- a/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go +++ b/api/pkg/apis/v1alpha1/vendors/targets-vendor_test.go @@ -9,6 +9,7 @@ package vendors import ( "context" "encoding/json" + "fmt" "testing" sym_mgr "github.com/eclipse-symphony/symphony/api/pkg/apis/v1alpha1/managers" @@ -31,9 +32,10 @@ import ( type MockCertManager struct{} func (m *MockCertManager) GetWorkingCert(ctx context.Context, targetName, namespace string) (string, string, error) { - // Return mock certificate data - public := "-----BEGIN CERTIFICATE----- mock-public-cert-data -----END CERTIFICATE-----" - private := "-----BEGIN PRIVATE KEY----- mock-private-key-data -----END PRIVATE KEY-----" + // Return mock secret reference strings instead of real PEM data to match MockSecretProvider behavior + secretName := fmt.Sprintf("%s-tls", targetName) + public := fmt.Sprintf("%s>>%s", secretName, "tls.crt") + private := fmt.Sprintf("%s>>%s", secretName, "tls.key") return public, private, nil } @@ -103,12 +105,8 @@ func createTargetsVendor() TargetsVendor { vendor.TargetsManager.TargetValidator = validation.NewTargetValidator(nil, nil) vendor.TargetsManager.SecretProvider = &secretProvider - // Set up mock CertManager - create a real CertManager but with mock providers - mockCertManager := &cert.CertManager{ - StateProvider: &stateProvider, - SecretProvider: &secretProvider, - } - vendor.CertManager = mockCertManager + // Set up mock CertManager - use the lightweight test mock so GetWorkingCert returns mock secret refs + vendor.CertManager = &MockCertManager{} return vendor }