From 80c5d18f84259d743311ccdfe05899d3b81de9f9 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Thu, 5 Nov 2026 14:24:25 -0500 Subject: [PATCH] Change Namespace.Spec.Project structure This commit makes some changes to the Namespace CR to make the handling of Temporal Cloud-returned identifiers more consistent and to provide extensibility to allow referencing Temporal Cloud resources by their Kubernetes Custom Resource counterpart. We modify `api/cloud/common.ResourceStatus` by shortening the name of the `ResourceVersion` field to just `Version`. This was done because `ResourceStatus` is now no longer an inlined field in `NamespaceStatus` but rather a named field inside the `NamespaceStatus` struct. `NamespaceStatus` struct now has another field called `References` which contains Temporal Cloud resource identifiers of any related resources -- e.g. Projects or Connectivity Rules: ```go // NamespaceStatus describes the current status of a Temporal Cloud namespace. // This struct does not contain a FromProto or ToProto function because there is // no corresponding NamespaceStatus struct in the Temporal Cloud API. // +k8s:openapi-gen=true type NamespaceStatus struct { tmprlcorev1.StatusBase `json:",inline"` // Resource contains common information about the Temporal Cloud resource // that this CR represents. Resource cloudcommon.ResourceStatus `json:"resource"` // References is an optional field containing the Temporal Cloud resource // IDs for any related resources. References NamespaceReferences `json:"references,omitempty"` } // NamespaceReferences is an optional field containing the // Temporal Cloud resource IDs for any related resources. type NamespaceReferences struct { // Project is the Temporal Cloud resource identifier for the Project // containing this Temporal Namespace. Project string `json:"project,omitempty"` // ConnectivityRules is the set of Temporal Cloud resource identifiers for // any Namespace ConnectivityRules associated with this Temporal Namespace ConnectivityRules []string `json:"connectivityRules,omitempty"` } ``` The `NamespaceSpec.Project` field type has been modified from `string` to `ProjectReferenceOrID`, which is a struct that can contain *either* a Temporal Cloud resource identifier *or* a reference to a (to be created) `cloud.temporal.io/Project` CR: ```go type ProjectReferenceOrID struct { // Ref is an optional field that points at a cloud.temporal.io/Project. // +optional Ref *ProjectReference `json:"ref,omitempty"` // ID is the Temporal Cloud Project resource identifier // +optional ID string `json:"id,omitempty"` } // ProjectReference points at a cloud.temporal.io/Project. type ProjectReference struct { // Namespace contains the optional Kubernetes Namespace in which to find // the Project. If empty, the Kubernetes Namespace containing the referring // object is used. Namespace string `json:"namespace,omitempty"` // Name is the name of the Project. // +required Name string `json:"name"` } ``` The reconciler's logic has been changed in the following important way: `Namespace.Spec.Project` is NEVER mutated once by the user. Prior to this commit, the `ProjectId` field in the `CreateNamespaceResponse` struct was used to populate the Spec.Project field. This is no longer the case. Instead, `ProjectId` is used to populate the `Status.References.Project` field, which is specifically storing the Temporal Cloud-returned resource identifier for the Temporal Cloud Project containing the Temporal Namespace. Anywhere that was previously checking for whether `Spec.Project` was set is now checking `Status.References.Project`. Issue #47 Signed-off-by: Jay Pipes --- api/cloud/common/resource_status.go | 20 ++- api/cloud/v1alpha1/namespace.go | 54 ++++-- api/cloud/v1alpha1/namespace_test.go | 115 ++++++------- api/cloud/v1alpha1/project_reference.go | 25 +++ api/cloud/v1alpha1/zz_generated.deepcopy.go | 66 +++++++- controller/namespace/delete.go | 64 +++++--- controller/namespace/delete_test.go | 14 +- controller/namespace/reconcile.go | 74 ++++++--- .../namespace/reconcile_internal_test.go | 4 +- controller/namespace/reconcile_test.go | 27 ++- .../bases/cloud.temporal.io_namespaces.yaml | 154 ++++++++++++------ 11 files changed, 414 insertions(+), 203 deletions(-) create mode 100644 api/cloud/v1alpha1/project_reference.go diff --git a/api/cloud/common/resource_status.go b/api/cloud/common/resource_status.go index 7a4637f..be38a22 100644 --- a/api/cloud/common/resource_status.go +++ b/api/cloud/common/resource_status.go @@ -17,20 +17,15 @@ type ResourceStatus struct { // Namespaces. // +optional ID string `json:"id,omitempty"` - // ResourceVersion is the version of the resource in Temporal Cloud. It is + // Version is the version of the resource in Temporal Cloud. It is // sent with each update and delete request. // Maps to Temporal Cloud proto `resource_version`. // +optional - ResourceVersion string `json:"resourceVersion,omitempty"` + Version string `json:"version,omitempty"` // State is the state of the resource in Temporal Cloud. // Maps to Temporal Cloud proto `state`. // +optional State ResourceState `json:"state,omitempty"` - // AsyncOperationID is the ID of the most recent asynchronous operation - // started on the resource. - // Maps to Temporal Cloud proto `async_operation_id`. - // +optional - AsyncOperationID string `json:"asyncOperationID,omitempty"` // CreatedTime is when the resource was created in Temporal Cloud. // Maps to Temporal Cloud proto `created_time`. // +optional @@ -40,6 +35,17 @@ type ResourceStatus struct { // Maps to Temporal Cloud proto `last_modified_time`. // +optional LastModifiedTime *metav1.Time `json:"lastModifiedTime,omitempty"` + // asyncOperationID is an optional field that contains the ID of the last + // async operation on this resource that was NOT in a terminal state + // (FAILED, COMPLETED, FULFILLED,REJECTED) when this reconcile loop + // executed. + // + // The reconciler removes this field when it determines the operation has + // reached a terminal state. + // + // Maps to Temporal Cloud proto `async_operation_id`. + // +optional + AsyncOperationID string `json:"asyncOperationID,omitempty"` } // ResourceState is the state of a resource in Temporal Cloud. diff --git a/api/cloud/v1alpha1/namespace.go b/api/cloud/v1alpha1/namespace.go index 7e101d2..33f2764 100644 --- a/api/cloud/v1alpha1/namespace.go +++ b/api/cloud/v1alpha1/namespace.go @@ -45,6 +45,8 @@ func (n Namespace) FromProto(in *tcloudnamespacev1.Namespace) *Namespace { TypeMeta: n.TypeMeta, ObjectMeta: n.ObjectMeta, } + out.Spec.Project = n.Spec.Project + out.Spec.Tags = maps.Clone(n.Spec.Tags) if in == nil { return &out @@ -52,16 +54,24 @@ func (n Namespace) FromProto(in *tcloudnamespacev1.Namespace) *Namespace { var spec NamespaceSpec out.Spec = spec.FromProto(in.GetSpec()) - out.Spec.Project = in.GetProjectId() + + // We use Status.Cloud.References.Project to save the project identifier + // returned from Temporal Cloud. We *never* mutate Spec.Project once it has + // been set. + out.Spec.Project = n.Spec.Project + + // But we *do* update the Spec.Tags to contain the collection of tags + // returned from Temporal Cloud. out.Spec.Tags = maps.Clone(in.GetTags()) - out.Status.ID = in.GetNamespace() - out.Status.ResourceVersion = in.GetResourceVersion() - out.Status.State.FromProto(in.GetState()) - out.Status.AsyncOperationID = in.GetAsyncOperationId() - out.Status.CreatedTime = transform.TimeFromProto(in.GetCreatedTime()) - out.Status.LastModifiedTime = transform.TimeFromProto(in.GetLastModifiedTime()) + out.Status.Resource.ID = in.GetNamespace() + out.Status.Resource.Version = in.GetResourceVersion() + out.Status.Resource.State.FromProto(in.GetState()) + out.Status.Resource.AsyncOperationID = in.GetAsyncOperationId() + out.Status.Resource.CreatedTime = transform.TimeFromProto(in.GetCreatedTime()) + out.Status.Resource.LastModifiedTime = transform.TimeFromProto(in.GetLastModifiedTime()) + out.Status.References.Project = in.GetProjectId() return &out } @@ -77,7 +87,7 @@ func (n Namespace) ToProto() *tcloudnamespacev1.Namespace { spec.Name = n.Name return &tcloudnamespacev1.Namespace{ Spec: spec, - ProjectId: n.Spec.Project, + ProjectId: n.Status.References.Project, Tags: maps.Clone(n.Spec.Tags), } } @@ -89,12 +99,10 @@ func (n Namespace) ToProto() *tcloudnamespacev1.Namespace { // +kubebuilder:validation:XValidation:rule="has(self.project) == has(oldSelf.project) && (!has(self.project) || self.project == oldSelf.project)",message="project cannot be added, changed or removed after creation",fieldPath=".project" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.searchAttributes) || (has(self.searchAttributes) && oldSelf.searchAttributes.all(k, k in self.searchAttributes && self.searchAttributes[k] == oldSelf.searchAttributes[k]))",message="searchAttributes cannot be removed or have their type changed",fieldPath=".searchAttributes" type NamespaceSpec struct { - // Project is the identifier of the Temporal Cloud project that the Namespace - // belongs to. - // Maps to Temporal Cloud proto `Namespace.project_id`. + // Project contains *either* a Temporal Cloud Project identifier string + // *or* a ProjectReference that points to a cloud.temporal.io/Project. // +optional - // +kubebuilder:validation:MinLength=1 - Project string `json:"project,omitempty"` + Project *ProjectReferenceOrID `json:"project,omitempty"` // Tags are key-value pairs to attach to the Namespace. // Maps to Temporal Cloud proto `Namespace.tags`. // +optional @@ -982,8 +990,24 @@ func (m EncryptionValidationMode) ToProto() tcloudnamespacev1.EncryptionValidati // no corresponding NamespaceStatus struct in the Temporal Cloud API. // +k8s:openapi-gen=true type NamespaceStatus struct { - tmprlcorev1.StatusBase `json:",inline"` - cloudcommon.ResourceStatus `json:",inline"` + tmprlcorev1.StatusBase `json:",inline"` + // Resource contains common information about the Temporal Cloud resource + // that this CR represents. + Resource cloudcommon.ResourceStatus `json:"resource"` + // References is an optional field containing the Temporal Cloud resource + // IDs for any related resources. + References NamespaceReferences `json:"references,omitempty"` +} + +// NamespaceReferences is an optional field containing the +// Temporal Cloud resource IDs for any related resources. +type NamespaceReferences struct { + // Project is the Temporal Cloud resource identifier for the Project + // containing this Temporal Namespace. + Project string `json:"project,omitempty"` + // ConnectivityRules is the set of Temporal Cloud resource identifiers for + // any Namespace ConnectivityRules associated with this Temporal Namespace + ConnectivityRules []string `json:"connectivityRules,omitempty"` } // NamespaceList is a list of Namespace resources diff --git a/api/cloud/v1alpha1/namespace_test.go b/api/cloud/v1alpha1/namespace_test.go index f5017c3..fa622d7 100644 --- a/api/cloud/v1alpha1/namespace_test.go +++ b/api/cloud/v1alpha1/namespace_test.go @@ -15,12 +15,25 @@ import ( "github.com/temporalio/cloud-operator/api/cloud/v1alpha1" ) +func newNamespace() v1alpha1.Namespace { + ns := v1alpha1.Namespace{} + ns.Namespace = "my-ns" + ns.Name = "my-name" + ns.Spec.Project = &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + } + ns.Spec.Tags = map[string]string{"a": "1"} + return ns +} + // fullSpec and fullProtoNamespace describe the same namespace with every // field set. Pairs of booleans are given different values so that a swapped // field mapping is caught even if both directions swap it. func fullSpec() v1alpha1.NamespaceSpec { return v1alpha1.NamespaceSpec{ - Project: "proj-1", + Project: &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + }, Tags: map[string]string{"team": "core", "env": "prod"}, Replicas: []v1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}, {Region: "aws-us-west-2"}}, RetentionDays: 30, @@ -142,19 +155,25 @@ func TestNamespaceFromProto_Spec(t *testing.T) { exp v1alpha1.NamespaceSpec }{ { - name: "nil proto yields empty spec", - in: nil, - exp: v1alpha1.NamespaceSpec{}, + name: "nil proto keeps project and tags fields", + exp: v1alpha1.NamespaceSpec{ + Project: &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + }, + Tags: map[string]string{"a": "1"}, + }, }, { - name: "nil proto spec keeps outer fields", + name: "nil proto spec keeps project field but overrides tags field", in: &tcloudnamespacev1.Namespace{ - ProjectId: "proj-1", - Tags: map[string]string{"a": "1"}, + ProjectId: "proj-other", + Tags: map[string]string{"team": "core", "env": "prod"}, }, exp: v1alpha1.NamespaceSpec{ - Project: "proj-1", - Tags: map[string]string{"a": "1"}, + Project: &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + }, + Tags: map[string]string{"team": "core", "env": "prod"}, }, }, { @@ -165,8 +184,13 @@ func TestNamespaceFromProto_Spec(t *testing.T) { Replicas: []*tcloudnamespacev1.ReplicaSpec{{Region: "aws-us-east-1"}}, RetentionDays: 7, }, + Tags: map[string]string{"a": "1"}, }, exp: v1alpha1.NamespaceSpec{ + Project: &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + }, + Tags: map[string]string{"a": "1"}, Replicas: []v1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}}, RetentionDays: 7, }, @@ -174,12 +198,21 @@ func TestNamespaceFromProto_Spec(t *testing.T) { { name: "every field", in: fullProtoNamespace(), - exp: fullSpec(), + exp: func() v1alpha1.NamespaceSpec { + ns := fullSpec() + // Project is never mutated. + ns.Project = &v1alpha1.ProjectReferenceOrID{ + ID: "proj-1", + } + // But tags can be mutated from proto. + ns.Tags = map[string]string{"team": "core", "env": "prod"} + return ns + }(), }, } for _, tc := range cases { t.Run(tc.name, func(tt *testing.T) { - var ns v1alpha1.Namespace + ns := newNamespace() got := ns.FromProto(tc.in) @@ -212,7 +245,7 @@ func TestNamespaceFromProto_Status(t *testing.T) { }, exp: cloudcommon.ResourceStatus{ ID: "my-ns.acct1", - ResourceVersion: "rv-1", + Version: "rv-1", State: cloudcommon.ResourceStateActive, AsyncOperationID: "op-1", CreatedTime: &metav1.Time{Time: created}, @@ -228,7 +261,7 @@ func TestNamespaceFromProto_Status(t *testing.T) { got := ns.FromProto(tc.in) - require.Equal(tc.exp, got.Status.ResourceStatus) + require.Equal(tc.exp, got.Status.Resource) // The qualified ID goes to status, never to metadata.name. require.Equal("my-ns", got.Name) }) @@ -256,7 +289,11 @@ func TestNamespaceToProto(t *testing.T) { // ToProto sets only the fields that are sent to Temporal Cloud, so the // qualified ID and status fields of the full proto are left unset. full := fullProtoNamespace() + + // The namespace and project ID are actually set from the Namespace CR's + // Metadata and Status, respectively. full.Namespace = "" + full.ProjectId = "" cases := []struct { name string @@ -291,58 +328,6 @@ func TestNamespaceToProto(t *testing.T) { } } -func TestNamespace_RoundTrip(t *testing.T) { - cases := []struct { - name string - spec v1alpha1.NamespaceSpec - }{ - { - name: "minimal", - spec: v1alpha1.NamespaceSpec{ - Replicas: []v1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}}, - RetentionDays: 7, - }, - }, - { - name: "every field", - spec: fullSpec(), - }, - { - name: "on-demand capacity", - spec: v1alpha1.NamespaceSpec{ - Replicas: []v1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}}, - RetentionDays: 7, - Capacity: &v1alpha1.CapacitySpec{OnDemand: &v1alpha1.OnDemandCapacity{}}, - }, - }, - { - name: "explicitly empty blocks stay non-nil", - spec: v1alpha1.NamespaceSpec{ - Replicas: []v1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}}, - RetentionDays: 7, - MTLSAuth: &v1alpha1.MTLSAuthSpec{}, - APIKeyAuth: &v1alpha1.APIKeyAuthSpec{}, - CodecServer: &v1alpha1.CodecServerSpec{CustomErrorMessage: &v1alpha1.CodecServerCustomErrorMessage{}}, - Lifecycle: &cloudcommon.LifecycleSpec{}, - HighAvailability: &v1alpha1.HighAvailabilitySpec{}, - Fairness: &v1alpha1.FairnessSpec{}, - EncryptionValidation: &v1alpha1.EncryptionValidationSpec{Mode: v1alpha1.EncryptionValidationModeDisabled}, - }, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(tt *testing.T) { - orig := namespaceWithSpec(tc.spec) - tp := orig.ToProto() - - var ns v1alpha1.Namespace - fp := ns.FromProto(tp) - - require.Equal(tt, tc.spec, fp.Spec) - }) - } -} - func TestCapacityFromProto(t *testing.T) { cases := []struct { name string diff --git a/api/cloud/v1alpha1/project_reference.go b/api/cloud/v1alpha1/project_reference.go new file mode 100644 index 0000000..0b80b7e --- /dev/null +++ b/api/cloud/v1alpha1/project_reference.go @@ -0,0 +1,25 @@ +package v1alpha1 + +// ProjectReferenceOrID contains *either* a Temporal Cloud Project identifier string +// *or* a ProjectReference that points to a cloud.temporal.io/Project. +// +// +kubebuilder:validation:XValidation:rule="has(self.ref) != has(self.id)",message="exactly one of ref or id must be set" +type ProjectReferenceOrID struct { + // Ref is an optional field that points at a cloud.temporal.io/Project. + // +optional + Ref *ProjectReference `json:"ref,omitempty"` + // ID is the Temporal Cloud Project resource identifier + // +optional + ID string `json:"id,omitempty"` +} + +// ProjectReference points at a cloud.temporal.io/Project. +type ProjectReference struct { + // Namespace contains the optional Kubernetes Namespace in which to find + // the Project. If empty, the Kubernetes Namespace containing the referring + // object is used. + Namespace string `json:"namespace,omitempty"` + // Name is the name of the Project. + // +required + Name string `json:"name"` +} diff --git a/api/cloud/v1alpha1/zz_generated.deepcopy.go b/api/cloud/v1alpha1/zz_generated.deepcopy.go index d4e253a..7fb084b 100644 --- a/api/cloud/v1alpha1/zz_generated.deepcopy.go +++ b/api/cloud/v1alpha1/zz_generated.deepcopy.go @@ -435,9 +435,35 @@ func (in *NamespaceList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NamespaceReferences) DeepCopyInto(out *NamespaceReferences) { + *out = *in + if in.ConnectivityRules != nil { + in, out := &in.ConnectivityRules, &out.ConnectivityRules + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceReferences. +func (in *NamespaceReferences) DeepCopy() *NamespaceReferences { + if in == nil { + return nil + } + out := new(NamespaceReferences) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespaceSpec) DeepCopyInto(out *NamespaceSpec) { *out = *in + if in.Project != nil { + in, out := &in.Project, &out.Project + *out = new(ProjectReferenceOrID) + (*in).DeepCopyInto(*out) + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make(map[string]string, len(*in)) @@ -519,7 +545,8 @@ func (in *NamespaceSpec) DeepCopy() *NamespaceSpec { func (in *NamespaceStatus) DeepCopyInto(out *NamespaceStatus) { *out = *in in.StatusBase.DeepCopyInto(&out.StatusBase) - in.ResourceStatus.DeepCopyInto(&out.ResourceStatus) + in.Resource.DeepCopyInto(&out.Resource) + in.References.DeepCopyInto(&out.References) return } @@ -549,6 +576,43 @@ func (in *OnDemandCapacity) DeepCopy() *OnDemandCapacity { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProjectReference) DeepCopyInto(out *ProjectReference) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProjectReference. +func (in *ProjectReference) DeepCopy() *ProjectReference { + if in == nil { + return nil + } + out := new(ProjectReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProjectReferenceOrID) DeepCopyInto(out *ProjectReferenceOrID) { + *out = *in + if in.Ref != nil { + in, out := &in.Ref, &out.Ref + *out = new(ProjectReference) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProjectReferenceOrID. +func (in *ProjectReferenceOrID) DeepCopy() *ProjectReferenceOrID { + if in == nil { + return nil + } + out := new(ProjectReferenceOrID) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProvisionedCapacity) DeepCopyInto(out *ProvisionedCapacity) { *out = *in diff --git a/controller/namespace/delete.go b/controller/namespace/delete.go index db95def..a86123f 100644 --- a/controller/namespace/delete.go +++ b/controller/namespace/delete.go @@ -46,7 +46,9 @@ func (r *Reconciler) ResourceDelete( latest := original.DeepCopy() cr := latest.CR().(*tcloudv1alpha1.Namespace) - if cr.Status.ID == "" { + + resourceID := cr.Status.Resource.ID + if resourceID == "" { // The user deleted the CR before the operator created a Namespace in // Temporal Cloud, so there's nothing to delete. return latest, nil @@ -70,7 +72,7 @@ func (r *Reconciler) ResourceDelete( if policy == tcloudcommon.DeletionPolicyRetain { log.Info( "retaining Temporal Cloud Namespace because of the retain deletion policy", - slog.String("namespace_id", cr.Status.ID), + slog.String("namespace_id", resourceID), slog.String("deletion_policy", string(policy)), ) return latest, nil @@ -82,30 +84,36 @@ func (r *Reconciler) ResourceDelete( return latest, err } + resourceState := cr.Status.Resource.State + asyncOpID := cr.Status.Resource.AsyncOperationID + // We may have already called DeleteNamespace for this resource. If so, // check whether the delete operation has finished. switch { - case cr.Status.State == tcloudcommon.ResourceStateDeleted: + case resourceState == tcloudcommon.ResourceStateDeleted: return latest, nil - case cr.Status.State == tcloudcommon.ResourceStateDeleteFailed: + case resourceState == tcloudcommon.ResourceStateDeleteFailed: // A previous delete failed. Don't send another one automatically. - err = ctrlerrors.NewTerminalError(fmt.Errorf( - "deleting Temporal Cloud namespace %s previously failed (async operation %s); not retrying", - cr.Status.ID, cr.Status.AsyncOperationID, - )) + err = ctrlerrors.NewTerminalError( + fmt.Errorf( + "deleting Temporal Cloud namespace %s previously failed "+ + "(async operation %s); not retrying", + resourceID, asyncOpID, + ), + ) return latest, err - case cr.Status.State == tcloudcommon.ResourceStateDeleting && - cr.Status.AsyncOperationID != "": + case resourceState == tcloudcommon.ResourceStateDeleting && + asyncOpID != "": err = checkDelete(ctx, tcc, latest, cr) return latest, err - case cr.Status.State == tcloudcommon.ResourceStateActivating, - cr.Status.State == tcloudcommon.ResourceStateUpdating: + case resourceState == tcloudcommon.ResourceStateActivating, + resourceState == tcloudcommon.ResourceStateUpdating: // A create or update on this custom resource is still in flight, and Temporal Cloud rejects a // delete until the Namespace is Active. Check again later. err = requeue.NeededAfter( fmt.Errorf( "waiting for tcloud namespace %s to finish %s before deleting", - cr.Status.ID, cr.Status.State, + resourceID, resourceState, ), requeue.DefaultRequeueAfterDuration, ) @@ -120,7 +128,7 @@ func (r *Reconciler) ResourceDelete( "tcloud namespace %s has deletion protection enabled; set "+ "spec.lifecycle.enableDeleteProtection to false to delete it, "+ "or set the %s annotation to %q to keep it", - cr.Status.ID, tcloudcommon.AnnotationKeyDeletionPolicy, + resourceID, tcloudcommon.AnnotationKeyDeletionPolicy, tcloudcommon.DeletionPolicyRetain, ) condition.SetStalled( @@ -133,7 +141,7 @@ func (r *Reconciler) ResourceDelete( // Read the namespace so deletion protection in Temporal Cloud can be made // to match the CR, and so the delete uses the current resource version. getResp, err := tcc.GetNamespace(ctx, &tcloudservicev1.GetNamespaceRequest{ - Namespace: cr.Status.ID, + Namespace: resourceID, }) if status.Code(err) == codes.NotFound { // Already gone. @@ -141,7 +149,7 @@ func (r *Reconciler) ResourceDelete( return latest, nil } if err != nil { - err = fmt.Errorf("failed getting tcloud namespace %s: %w", cr.Status.ID, err) + err = fmt.Errorf("failed getting tcloud namespace %s: %w", resourceID, err) return latest, err } tns := getResp.GetNamespace() @@ -154,7 +162,7 @@ func (r *Reconciler) ResourceDelete( } resp, err := tcc.DeleteNamespace(ctx, &tcloudservicev1.DeleteNamespaceRequest{ - Namespace: cr.Status.ID, + Namespace: resourceID, ResourceVersion: tns.GetResourceVersion(), }) if status.Code(err) == codes.NotFound { @@ -165,19 +173,22 @@ func (r *Reconciler) ResourceDelete( if err != nil { // The gRPC error includes Temporal Cloud's message, for example that // deletion protection is enabled. - err = fmt.Errorf("failed deleting tcloud namespace %s: %w", cr.Status.ID, err) + err = fmt.Errorf( + "failed deleting tcloud namespace %s: %w", + resourceID, err, + ) condition.SetStalled( latest, metav1.ConditionTrue, err.Error(), tcloudcommon.ReasonResourceDeleteFailed, ) return latest, err } - cr.Status.State = tcloudcommon.ResourceStateDeleting - cr.Status.AsyncOperationID = resp.GetAsyncOperation().GetId() + cr.Status.Resource.State = tcloudcommon.ResourceStateDeleting + cr.Status.Resource.AsyncOperationID = resp.GetAsyncOperation().GetId() err = requeue.NeededAfter( fmt.Errorf( "deleting tcloud namespace %s (async operation %s)", - cr.Status.ID, cr.Status.AsyncOperationID, + resourceID, asyncOpID, ), requeue.DefaultRequeueAfterDuration, ) @@ -224,13 +235,16 @@ func checkDelete( latest types.Resource, cr *tcloudv1alpha1.Namespace, ) error { + resourceID := cr.Status.Resource.ID + asyncOpID := cr.Status.Resource.AsyncOperationID + resp, err := tcc.GetAsyncOperation(ctx, &tcloudservicev1.GetAsyncOperationRequest{ - AsyncOperationId: cr.Status.AsyncOperationID, + AsyncOperationId: asyncOpID, }) if err != nil { return fmt.Errorf( "failed getting tcloud async operation %s: %w", - cr.Status.AsyncOperationID, err, + asyncOpID, err, ) } op := resp.GetAsyncOperation() @@ -242,12 +256,12 @@ func checkDelete( operationv1.AsyncOperation_STATE_CANCELLED: msg := fmt.Sprintf( "deleting tcloud namespace %s failed: async operation %s is %s: %s", - cr.Status.ID, op.GetId(), op.GetState(), op.GetFailureReason(), + resourceID, op.GetId(), op.GetState(), op.GetFailureReason(), ) condition.SetStalled( latest, metav1.ConditionTrue, msg, tcloudcommon.ReasonResourceDeleteFailed, ) - cr.Status.State = tcloudcommon.ResourceStateDeleteFailed + cr.Status.Resource.State = tcloudcommon.ResourceStateDeleteFailed return ctrlerrors.NewTerminalError(errors.New(msg)) default: // Pending, in progress, or a state this SDK doesn't know. diff --git a/controller/namespace/delete_test.go b/controller/namespace/delete_test.go index 81c6a0f..fef1761 100644 --- a/controller/namespace/delete_test.go +++ b/controller/namespace/delete_test.go @@ -40,21 +40,21 @@ const ( var ( statusActive = tcloudcommon.ResourceStatus{ - ID: deleteTestID, - ResourceVersion: deleteTestRV, - State: tcloudcommon.ResourceStateActive, + ID: deleteTestID, + Version: deleteTestRV, + State: tcloudcommon.ResourceStateActive, } statusDeleting = tcloudcommon.ResourceStatus{ ID: deleteTestID, - ResourceVersion: deleteTestRV, + Version: deleteTestRV, State: tcloudcommon.ResourceStateDeleting, AsyncOperationID: deleteTestAsyncOpID, } statusDeleteFailed = tcloudcommon.ResourceStatus{ ID: deleteTestID, - ResourceVersion: deleteTestRV, + Version: deleteTestRV, State: tcloudcommon.ResourceStateDeleteFailed, AsyncOperationID: deleteTestAsyncOpID, } @@ -377,7 +377,7 @@ func runDeleteCase(t *testing.T, tc deleteCase) { if tc.expStatus != nil { expStatus = *tc.expStatus } - require.Equal(expStatus, gotCR.Status.ResourceStatus) + require.Equal(expStatus, gotCR.Status.Resource) requireStalled(t, got, tc.expStalled) cloud.requireCalls(t, tc) @@ -399,7 +399,7 @@ func newDeleteTestNamespace( tcloudcommon.AnnotationKeyDeletionPolicy: policy, } } - tns.Status.ResourceStatus = rs + tns.Status.Resource = rs if protected { tns.Spec.Lifecycle = &tcloudcommon.LifecycleSpec{ EnableDeleteProtection: true, diff --git a/controller/namespace/reconcile.go b/controller/namespace/reconcile.go index 231df22..4a04845 100644 --- a/controller/namespace/reconcile.go +++ b/controller/namespace/reconcile.go @@ -115,8 +115,10 @@ func (r *Reconciler) ResourceSetLatest( // `latest` parameter. cr := latest.(*res).cr + resourceID := cr.Status.Resource.ID + var tns *tcloudnamespacev1.Namespace - if cr.Status.ID == "" { + if resourceID == "" { // The qualified namespace name is only known once a reconcile after // the create has saved it to status, and status can be lost (e.g. the // CR was recreated). Look the namespace up by name so that a duplicate @@ -128,7 +130,7 @@ func (r *Reconciler) ResourceSetLatest( } else { var resp *tcloudservicev1.GetNamespaceResponse resp, err = tcc.GetNamespace(ctx, &tcloudservicev1.GetNamespaceRequest{ - Namespace: cr.Status.ID, + Namespace: resourceID, }) if status.Code(err) == codes.NotFound { // The namespace was deleted outside of the operator. Treat it @@ -145,7 +147,7 @@ func (r *Reconciler) ResourceSetLatest( // The namespace does not exist in Temporal Cloud, so mutate latest // spec and status to be empty. cr.Spec = tcloudv1alpha1.NamespaceSpec{} - cr.Status.ResourceStatus = cloudcommon.ResourceStatus{} + cr.Status = tcloudv1alpha1.NamespaceStatus{} return nil } @@ -201,7 +203,9 @@ func syncNamespace( desired *tcloudv1alpha1.Namespace, latest *tcloudv1alpha1.Namespace, ) error { - if latest.Status.ID == "" { + resourceID := latest.Status.Resource.ID + + if resourceID == "" { tns := desired.ToProto() resp, err := tcc.CreateNamespace(ctx, &tcloudservicev1.CreateNamespaceRequest{ Spec: tns.GetSpec(), @@ -220,12 +224,15 @@ func syncNamespace( // Only an Active namespace accepts new operations. Every other state // either resolves on its own, so we wait, or needs someone to step in, so // we stop. Unknown states also wait rather than fall through to an update. - id := latest.Status.ID - switch state := latest.Status.State; state { + resourceState := latest.Status.Resource.State + switch resourceState { case cloudcommon.ResourceStateActive: case cloudcommon.ResourceStateActivating, cloudcommon.ResourceStateUpdating: return requeue.NeededAfter( - fmt.Errorf("waiting for tcloud namespace %q to leave state %s", id, state), + fmt.Errorf( + "waiting for tcloud namespace %q to leave state %s", + resourceID, resourceState, + ), requeue.DefaultRequeueAfterDuration, ) case cloudcommon.ResourceStateDeleting, cloudcommon.ResourceStateDeleted: @@ -233,7 +240,10 @@ func syncNamespace( // Cloud reports it as not found, ResourceSetLatest clears the status // and the next sync recreates it. return requeue.NeededAfter( - fmt.Errorf("waiting for tcloud namespace %q to finish deleting, state %s", id, state), + fmt.Errorf( + "waiting for tcloud namespace %q to finish deleting, state %s", + resourceID, resourceState, + ), requeue.DefaultRequeueAfterDuration, ) case cloudcommon.ResourceStateActivationFailed, @@ -241,13 +251,19 @@ func syncNamespace( cloudcommon.ResourceStateDeleteFailed, cloudcommon.ResourceStateSuspended, cloudcommon.ResourceStateExpired: - return ctrlerrors.NewTerminalError(fmt.Errorf( - "%w: tcloud namespace %q is in state %s, contact Temporal Cloud support", - ctrlerrors.ErrResourceSync, id, state, - )) + return ctrlerrors.NewTerminalError( + fmt.Errorf( + "%w: tcloud namespace %q is in state %s, "+ + "contact Temporal Cloud support", + ctrlerrors.ErrResourceSync, resourceID, resourceState, + ), + ) default: return requeue.NeededAfter( - fmt.Errorf("waiting for tcloud namespace %q to leave unknown state %q", id, state), + fmt.Errorf( + "waiting for tcloud namespace %q to leave unknown state %q", + resourceID, resourceState, + ), requeue.DefaultRequeueAfterDuration, ) } @@ -255,11 +271,15 @@ func syncNamespace( // spec.project is immutable on the CR, so a mismatch means the namespace // was moved outside of the operator. An empty spec.project means the // account's default project, whose ID Temporal Cloud reports back. - if desired.Spec.Project != "" && desired.Spec.Project != latest.Spec.Project { - return ctrlerrors.NewTerminalError(fmt.Errorf( - "%w: tcloud namespace %q is in project %q but spec.project is %q", - ctrlerrors.ErrResourceSync, id, latest.Spec.Project, desired.Spec.Project, - )) + if desired.Spec.Project.ID != "" && desired.Spec.Project != latest.Spec.Project { + return ctrlerrors.NewTerminalError( + fmt.Errorf( + "%w: tcloud namespace %q is in project %q "+ + "but spec.project.id is %q", + ctrlerrors.ErrResourceSync, resourceID, + latest.Spec.Project.ID, desired.Spec.Project.ID, + ), + ) } // The controller's DeepEqual always reports a difference @@ -271,15 +291,15 @@ func syncNamespace( specWithoutProjectAndTags(&latest.Spec), ) { _, err := tcc.UpdateNamespace(ctx, &tcloudservicev1.UpdateNamespaceRequest{ - Namespace: id, + Namespace: resourceID, Spec: desired.ToProto().GetSpec(), - ResourceVersion: latest.Status.ResourceVersion, + ResourceVersion: latest.Status.Resource.Version, }) if err != nil { return cloudCallError("updating tcloud namespace", err) } return requeue.NeededAfter( - fmt.Errorf("updated tcloud namespace %q, waiting for update to complete", id), + fmt.Errorf("updated tcloud namespace %q, waiting for update to complete", resourceID), requeue.DefaultRequeueAfterDuration, ) } @@ -287,7 +307,7 @@ func syncNamespace( upsert, remove := tagsDiff(desired.Spec.Tags, latest.Spec.Tags) if len(upsert) > 0 || len(remove) > 0 { _, err := tcc.UpdateNamespaceTags(ctx, &tcloudservicev1.UpdateNamespaceTagsRequest{ - Namespace: id, + Namespace: resourceID, TagsToUpsert: upsert, TagsToRemove: remove, }) @@ -295,7 +315,11 @@ func syncNamespace( return cloudCallError("updating tcloud namespace tags", err) } return requeue.NeededAfter( - fmt.Errorf("updated tcloud namespace %q tags, waiting for update to complete", id), + fmt.Errorf( + "updated tcloud namespace %q tags, "+ + "waiting for update to complete", + resourceID, + ), requeue.DefaultRequeueAfterDuration, ) } @@ -309,7 +333,9 @@ func specWithoutProjectAndTags( spec *tcloudv1alpha1.NamespaceSpec, ) *tcloudv1alpha1.NamespaceSpec { out := spec.DeepCopy() - out.Project = "" + out.Project = &tcloudv1alpha1.ProjectReferenceOrID{ + ID: "", + } out.Tags = nil if cs := out.CodecServer; cs != nil && cs.CustomErrorMessage != nil { cs.CustomErrorMessage.Default = nilIfEmpty(cs.CustomErrorMessage.Default) diff --git a/controller/namespace/reconcile_internal_test.go b/controller/namespace/reconcile_internal_test.go index 61575b4..3380938 100644 --- a/controller/namespace/reconcile_internal_test.go +++ b/controller/namespace/reconcile_internal_test.go @@ -243,7 +243,9 @@ func TestSpecWithoutProjectAndTags(t *testing.T) { name: "project and tags are ignored", desired: minimalSpec(), latest: func(s *tcloudv1alpha1.NamespaceSpec) { - s.Project = "project-1" + s.Project = &tcloudv1alpha1.ProjectReferenceOrID{ + ID: "project-1", + } s.Tags = map[string]string{"team": "payments"} }, expEqual: true, diff --git a/controller/namespace/reconcile_test.go b/controller/namespace/reconcile_test.go index c5a22e4..6975099 100644 --- a/controller/namespace/reconcile_test.go +++ b/controller/namespace/reconcile_test.go @@ -101,14 +101,19 @@ func TestReconciler_ResourceSetLatest(t *testing.T) { tns := &tcloudv1alpha1.Namespace{} tns.Name = tnsName tns.Namespace = kubeNamespace - tns.Spec.Project = project + tns.Spec.Project = &tcloudv1alpha1.ProjectReferenceOrID{ + ID: project, + } tns.Spec.RetentionDays = 7 // tnsSynced has been synced before, so ResourceSetLatest gets the // namespace by its Status.ID. tnsSynced := tns.DeepCopy() - tnsSynced.Status.ID = tnsID - tnsSynced.Status.ResourceVersion = "rv-0" + tnsSynced.Status.Resource.ID = tnsID + tnsSynced.Status.Resource.Version = "rv-0" + tnsSynced.Status.References = tcloudv1alpha1.NamespaceReferences{ + Project: project, + } // protoOutOfSync is the namespace as Temporal Cloud reports it. Its project // differs from the CR's so we can see that latest is mutated to match @@ -129,18 +134,26 @@ func TestReconciler_ResourceSetLatest(t *testing.T) { nsOutOfSync := tns.DeepCopy() nsOutOfSync.Spec = tcloudv1alpha1.NamespaceSpec{ - Project: projectOther, + // NOTE(jaypipes): The spec's project is never mutated once set. + // Instead, Status.Cloud.References.Project is set to the value + // returned from Temporal Cloud API. + Project: &tcloudv1alpha1.ProjectReferenceOrID{ + ID: project, + }, Tags: map[string]string{"team": "core"}, Replicas: []tcloudv1alpha1.ReplicaSpec{{Region: "aws-us-east-1"}}, RetentionDays: 30, } nsOutOfSync.Status = tcloudv1alpha1.NamespaceStatus{ - ResourceStatus: cloudcommon.ResourceStatus{ + Resource: cloudcommon.ResourceStatus{ ID: tnsID, - ResourceVersion: "rv-1", + Version: "rv-1", State: cloudcommon.ResourceStateActive, AsyncOperationID: "op-1", }, + References: tcloudv1alpha1.NamespaceReferences{ + Project: projectOther, + }, } // cleared empties the spec and status, which is what ResourceSetLatest @@ -148,7 +161,7 @@ func TestReconciler_ResourceSetLatest(t *testing.T) { cleared := tns.DeepCopy() cleared.Spec = tcloudv1alpha1.NamespaceSpec{} cleared.Status = tcloudv1alpha1.NamespaceStatus{ - ResourceStatus: cloudcommon.ResourceStatus{}, + Resource: cloudcommon.ResourceStatus{}, } cases := []struct { diff --git a/deploy/kustomize/crd/bases/cloud.temporal.io_namespaces.yaml b/deploy/kustomize/crd/bases/cloud.temporal.io_namespaces.yaml index e4cc6a4..cefd6f7 100644 --- a/deploy/kustomize/crd/bases/cloud.temporal.io_namespaces.yaml +++ b/deploy/kustomize/crd/bases/cloud.temporal.io_namespaces.yaml @@ -285,11 +285,31 @@ spec: type: object project: description: |- - Project is the identifier of the Temporal Cloud project that the Namespace - belongs to. - Maps to Temporal Cloud proto `Namespace.project_id`. - minLength: 1 - type: string + Project contains *either* a Temporal Cloud Project identifier string + *or* a ProjectReference that points to a cloud.temporal.io/Project. + properties: + id: + description: ID is the Temporal Cloud Project resource identifier + type: string + ref: + description: Ref is an optional field that points at a cloud.temporal.io/Project. + properties: + name: + description: Name is the name of the Project. + type: string + namespace: + description: |- + Namespace contains the optional Kubernetes Namespace in which to find + the Project. If empty, the Kubernetes Namespace containing the referring + object is used. + type: string + required: + - name + type: object + type: object + x-kubernetes-validations: + - message: exactly one of ref or id must be set + rule: has(self.ref) != has(self.id) replicas: description: |- Replicas are the regions where the Namespace is placed. @@ -360,12 +380,6 @@ spec: status: description: Status describes the current status of a Namespace. properties: - asyncOperationID: - description: |- - AsyncOperationID is the ID of the most recent asynchronous operation - started on the resource. - Maps to Temporal Cloud proto `async_operation_id`. - type: string conditions: description: Conditions is the collection of Conditions associated with the resource. @@ -427,48 +441,86 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - createdTime: + references: description: |- - CreatedTime is when the resource was created in Temporal Cloud. - Maps to Temporal Cloud proto `created_time`. - format: date-time - type: string - id: - description: |- - ID is the identifier Temporal Cloud uses for the resource. Not set for - resources that Temporal Cloud identifies by name. - Maps to Temporal Cloud proto `id`, or `Namespace.namespace` for - Namespaces. - type: string - lastModifiedTime: - description: |- - LastModifiedTime is when the resource was last modified in Temporal - Cloud. - Maps to Temporal Cloud proto `last_modified_time`. - format: date-time - type: string - resourceVersion: - description: |- - ResourceVersion is the version of the resource in Temporal Cloud. It is - sent with each update and delete request. - Maps to Temporal Cloud proto `resource_version`. - type: string - state: + References is an optional field containing the Temporal Cloud resource + IDs for any related resources. + properties: + connectivityRules: + description: |- + ConnectivityRules is the set of Temporal Cloud resource identifiers for + any Namespace ConnectivityRules associated with this Temporal Namespace + items: + type: string + type: array + project: + description: |- + Project is the Temporal Cloud resource identifier for the Project + containing this Temporal Namespace. + type: string + type: object + resource: description: |- - State is the state of the resource in Temporal Cloud. - Maps to Temporal Cloud proto `state`. - enum: - - Activating - - ActivationFailed - - Active - - Updating - - UpdateFailed - - Deleting - - DeleteFailed - - Deleted - - Suspended - - Expired - type: string + Resource contains common information about the Temporal Cloud resource + that this CR represents. + properties: + asyncOperationID: + description: |- + asyncOperationID is an optional field that contains the ID of the last + async operation on this resource that was NOT in a terminal state + (FAILED, COMPLETED, FULFILLED,REJECTED) when this reconcile loop + executed. + + The reconciler removes this field when it determines the operation has + reached a terminal state. + + Maps to Temporal Cloud proto `async_operation_id`. + type: string + createdTime: + description: |- + CreatedTime is when the resource was created in Temporal Cloud. + Maps to Temporal Cloud proto `created_time`. + format: date-time + type: string + id: + description: |- + ID is the identifier Temporal Cloud uses for the resource. Not set for + resources that Temporal Cloud identifies by name. + Maps to Temporal Cloud proto `id`, or `Namespace.namespace` for + Namespaces. + type: string + lastModifiedTime: + description: |- + LastModifiedTime is when the resource was last modified in Temporal + Cloud. + Maps to Temporal Cloud proto `last_modified_time`. + format: date-time + type: string + state: + description: |- + State is the state of the resource in Temporal Cloud. + Maps to Temporal Cloud proto `state`. + enum: + - Activating + - ActivationFailed + - Active + - Updating + - UpdateFailed + - Deleting + - DeleteFailed + - Deleted + - Suspended + - Expired + type: string + version: + description: |- + Version is the version of the resource in Temporal Cloud. It is + sent with each update and delete request. + Maps to Temporal Cloud proto `resource_version`. + type: string + type: object + required: + - resource type: object required: - spec