From 19029a3168a36a234e63682c3f55b80c8a0b24f3 Mon Sep 17 00:00:00 2001 From: Aleksandr Rybolovlev Date: Thu, 17 Oct 2024 19:25:13 +0200 Subject: [PATCH 1/3] Update Dockerfile with a new Red Hat UBI image target (#492) --- Dockerfile | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Dockerfile b/Dockerfile index cd77a05a..51f864f3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -89,6 +89,36 @@ USER 65532:65532 ENTRYPOINT ["/bin/sh", "-c", "/$BIN_NAME"] +# Red Hat UBI release image +# ----------------------------------- +FROM registry.access.redhat.com/ubi9/ubi-micro:9.4-15 AS release-ubi + +ARG BIN_NAME +ARG PRODUCT_VERSION +ARG PRODUCT_REVISION +ARG TARGETOS +ARG TARGETARCH + +ENV BIN_NAME=$BIN_NAME + +LABEL name="HCP Terraform Operator" +LABEL vendor="HashiCorp" +LABEL release=$PRODUCT_REVISION +LABEL summary="HCP Terraform Operator for Kubernetes allows managing HCP Terraform / Terraform Enterprise resources via Kubernetes Custom Resources" +LABEL description="HCP Terraform Operator for Kubernetes allows managing HCP Terraform / Terraform Enterprise resources via Kubernetes Custom Resources" + +LABEL maintainer="Terraform Ecosystem - Hybrid Cloud Team " +LABEL version=$PRODUCT_VERSION +LABEL revision=$PRODUCT_REVISION + +WORKDIR / +COPY LICENSE /licenses/copyright.txt +COPY dist/$TARGETOS/$TARGETARCH/$BIN_NAME . + +USER 65532:65532 + +ENTRYPOINT ["/bin/sh", "-c", "/$BIN_NAME"] + # =================================== # # Set default target to 'dev'. From 83479af2a0300b2f7456741e486008a47c5a6f85 Mon Sep 17 00:00:00 2001 From: Aleksandr Rybolovlev Date: Fri, 18 Oct 2024 10:26:04 +0200 Subject: [PATCH 2/3] Update Workspace Notifications reconciliation logic (#477) --- .../ENHANCEMENTS-477-20240821-134240.yaml | 5 + .../workspace_controller_notifications.go | 279 +++++++----------- ...workspace_controller_notifications_test.go | 3 +- 3 files changed, 116 insertions(+), 171 deletions(-) create mode 100644 .changes/unreleased/ENHANCEMENTS-477-20240821-134240.yaml diff --git a/.changes/unreleased/ENHANCEMENTS-477-20240821-134240.yaml b/.changes/unreleased/ENHANCEMENTS-477-20240821-134240.yaml new file mode 100644 index 00000000..9be5b12f --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-477-20240821-134240.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: '`Workspace`: Update Notifications reconciliation logic to reduce the number of API calls.' +time: 2024-08-21T13:42:40.942036+02:00 +custom: + PR: "477" diff --git a/controllers/workspace_controller_notifications.go b/controllers/workspace_controller_notifications.go index 9e609e6b..4f448dd3 100644 --- a/controllers/workspace_controller_notifications.go +++ b/controllers/workspace_controller_notifications.go @@ -14,46 +14,26 @@ import ( "github.com/hashicorp/hcp-terraform-operator/internal/slice" ) -func hasNotificationItem(s []tfc.NotificationConfiguration, f tfc.NotificationConfiguration) (int, bool) { - for i, v := range s { - if cmp.Equal(v, f, cmpopts.IgnoreFields(tfc.NotificationConfiguration{}, - "CreatedAt", "DeliveryResponses", "EmailAddresses", "EmailUsers", "Enabled", "ID", "Subscribable", "Token", "Triggers", "URL", "UpdatedAt")) { +func hasNotificationItem(notificaions []tfc.NotificationConfiguration, notificaion tfc.NotificationConfiguration) (int, bool) { + for i, n := range notificaions { + if notificaion.Name == n.Name && notificaion.DestinationType == n.DestinationType { return i, true } - } return -1, false } -func notificationsDifference(a, b []tfc.NotificationConfiguration) []tfc.NotificationConfiguration { - var d []tfc.NotificationConfiguration - bc := make([]tfc.NotificationConfiguration, len(b)) - - copy(bc, b) - - for _, av := range a { - i, t := hasNotificationItem(bc, av) - if t { - bc = slice.RemoveFromSlice(bc, i) - } else { - d = append(d, av) - } - } - - return d -} - func (r *WorkspaceReconciler) getOrgMembers(ctx context.Context, w *workspaceInstance) (map[string]*tfc.User, error) { - eu := make(map[string]*tfc.User) - e := make([]string, 0) + emailUsers := make(map[string]*tfc.User) + emails := make([]string, 0) for _, n := range w.instance.Spec.Notifications { - for _, ne := range n.EmailUsers { - eu[ne] = nil - e = append(e, ne) - } + emails = append(emails, n.EmailUsers...) + } + if len(emails) == 0 { + return emailUsers, nil } listOpts := &tfc.OrganizationMembershipListOptions{ - Emails: e, + Emails: emails, ListOptions: tfc.ListOptions{ PageSize: maxPageSize, }, @@ -64,14 +44,14 @@ func (r *WorkspaceReconciler) getOrgMembers(ctx context.Context, w *workspaceIns return nil, err } for _, m := range members.Items { - eu[m.Email] = &tfc.User{ID: m.User.ID} + emailUsers[m.Email] = &tfc.User{ID: m.User.ID} } if members.NextPage == 0 { break } listOpts.PageNumber = members.NextPage } - return eu, nil + return emailUsers, nil } func (r *WorkspaceReconciler) getInstanceNotifications(ctx context.Context, w *workspaceInstance) ([]tfc.NotificationConfiguration, error) { @@ -84,36 +64,39 @@ func (r *WorkspaceReconciler) getInstanceNotifications(ctx context.Context, w *w return []tfc.NotificationConfiguration{}, err } - o := make([]tfc.NotificationConfiguration, len(w.instance.Spec.Notifications)) + notifications := make([]tfc.NotificationConfiguration, len(w.instance.Spec.Notifications)) - for i, n := range w.instance.Spec.Notifications { - var eu []*tfc.User - for _, e := range n.EmailUsers { - if v, ok := orgEmailUsers[e]; ok && v != nil { - eu = append(eu, v) + for i, notification := range w.instance.Spec.Notifications { + var emailUsers []*tfc.User + for _, emailUser := range notification.EmailUsers { + if v, ok := orgEmailUsers[emailUser]; ok { + emailUsers = append(emailUsers, v) + } else { + w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("user %s was not found", emailUser)) + return []tfc.NotificationConfiguration{}, fmt.Errorf("user %s was not found", emailUser) } } - nt := make([]string, len(n.Triggers)) - for i, t := range n.Triggers { - nt[i] = string(t) + triggers := make([]string, len(notification.Triggers)) + for i, t := range notification.Triggers { + triggers[i] = string(t) } - o[i] = tfc.NotificationConfiguration{ - Name: n.Name, - DestinationType: n.Type, - URL: n.URL, - Enabled: n.Enabled, - Token: n.Token, - Triggers: nt, - EmailAddresses: n.EmailAddresses, - EmailUsers: eu, + notifications[i] = tfc.NotificationConfiguration{ + Name: notification.Name, + DestinationType: notification.Type, + URL: notification.URL, + Enabled: notification.Enabled, + Token: notification.Token, + Triggers: triggers, + EmailAddresses: notification.EmailAddresses, + EmailUsers: emailUsers, } } - return o, nil + return notifications, nil } func (r *WorkspaceReconciler) getWorkspaceNotifications(ctx context.Context, w *workspaceInstance) ([]tfc.NotificationConfiguration, error) { - var o []tfc.NotificationConfiguration + var notifications []tfc.NotificationConfiguration listOpts := &tfc.NotificationConfigurationListOptions{ ListOptions: tfc.ListOptions{ @@ -125,17 +108,17 @@ func (r *WorkspaceReconciler) getWorkspaceNotifications(ctx context.Context, w * if err != nil { return nil, err } - for _, n := range wn.Items { - o = append(o, tfc.NotificationConfiguration{ - ID: n.ID, - Name: n.Name, - DestinationType: n.DestinationType, - URL: n.URL, - Enabled: n.Enabled, - Token: n.Token, - Triggers: n.Triggers, - EmailAddresses: n.EmailAddresses, - EmailUsers: n.EmailUsers, + for _, notification := range wn.Items { + notifications = append(notifications, tfc.NotificationConfiguration{ + ID: notification.ID, + Name: notification.Name, + DestinationType: notification.DestinationType, + URL: notification.URL, + Enabled: notification.Enabled, + Token: notification.Token, + Triggers: notification.Triggers, + EmailAddresses: notification.EmailAddresses, + EmailUsers: notification.EmailUsers, }) } if wn.NextPage == 0 { @@ -144,83 +127,51 @@ func (r *WorkspaceReconciler) getWorkspaceNotifications(ctx context.Context, w * listOpts.PageNumber = wn.NextPage } - return o, nil -} - -func getNotificationsToCreate(spec, ws []tfc.NotificationConfiguration) []tfc.NotificationConfiguration { - return notificationsDifference(spec, ws) -} - -func getNotificationsToUpdate(spec, ws []tfc.NotificationConfiguration) []tfc.NotificationConfiguration { - o := []tfc.NotificationConfiguration{} - - if len(spec) == 0 || len(ws) == 0 { - return o - } - - for _, sv := range spec { - for _, wv := range ws { - if sv.Name == wv.Name { - if !cmp.Equal(sv, wv, cmpopts.IgnoreFields(tfc.NotificationConfiguration{}, - "CreatedAt", "DeliveryResponses", "ID", "Subscribable", "UpdatedAt")) { - sv.ID = wv.ID - o = append(o, sv) - } - } - } - } - - return o -} - -func getNotificationsToDelete(spec, ws []tfc.NotificationConfiguration) []tfc.NotificationConfiguration { - return notificationsDifference(ws, spec) + return notifications, nil } -func (r *WorkspaceReconciler) createNotifications(ctx context.Context, w *workspaceInstance, create []tfc.NotificationConfiguration) error { - for _, c := range create { - w.log.Info("Reconcile Notifications", "msg", "creating notificaion") - nt := make([]tfc.NotificationTriggerType, len(c.Triggers)) - for i, t := range c.Triggers { - nt[i] = tfc.NotificationTriggerType(t) - } - _, err := w.tfClient.Client.NotificationConfigurations.Create(ctx, w.instance.Status.WorkspaceID, tfc.NotificationConfigurationCreateOptions{ - Name: &c.Name, - DestinationType: &c.DestinationType, - URL: &c.URL, - Enabled: &c.Enabled, - Token: &c.Token, - Triggers: nt, - EmailUsers: c.EmailUsers, - EmailAddresses: c.EmailAddresses, - }) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to create a new notification %q", c.ID)) - return err - } +func (w *workspaceInstance) createNotification(ctx context.Context, notificaion tfc.NotificationConfiguration) error { + w.log.Info("Reconcile Notifications", "msg", "creating notificaion") + nt := make([]tfc.NotificationTriggerType, len(notificaion.Triggers)) + for i, t := range notificaion.Triggers { + nt[i] = tfc.NotificationTriggerType(t) + } + _, err := w.tfClient.Client.NotificationConfigurations.Create(ctx, w.instance.Status.WorkspaceID, tfc.NotificationConfigurationCreateOptions{ + Name: ¬ificaion.Name, + DestinationType: ¬ificaion.DestinationType, + URL: ¬ificaion.URL, + Enabled: ¬ificaion.Enabled, + Token: ¬ificaion.Token, + Triggers: nt, + EmailUsers: notificaion.EmailUsers, + EmailAddresses: notificaion.EmailAddresses, + }) + if err != nil { + w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to create a new notification %q", notificaion.ID)) + return err } return nil } -func (r *WorkspaceReconciler) updateNotifications(ctx context.Context, w *workspaceInstance, update []tfc.NotificationConfiguration) error { - for _, u := range update { - w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("updating notificaion %q", u.ID)) - nt := make([]tfc.NotificationTriggerType, len(u.Triggers)) - for i, t := range u.Triggers { - nt[i] = tfc.NotificationTriggerType(t) - } - _, err := w.tfClient.Client.NotificationConfigurations.Update(ctx, u.ID, tfc.NotificationConfigurationUpdateOptions{ - Name: &u.Name, - Enabled: &u.Enabled, - Token: &u.Token, - Triggers: nt, - URL: &u.URL, - EmailAddresses: u.EmailAddresses, - EmailUsers: u.EmailUsers, +func (w *workspaceInstance) updateNotification(ctx context.Context, sn, wn tfc.NotificationConfiguration) error { + if !cmp.Equal(sn, wn, cmpopts.IgnoreFields(tfc.NotificationConfiguration{}, "ID", "CreatedAt", "DeliveryResponses", "UpdatedAt", "Subscribable")) { + w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("updating notificaion %q", wn.ID)) + triggers := make([]tfc.NotificationTriggerType, len(sn.Triggers)) + for i, t := range sn.Triggers { + triggers[i] = tfc.NotificationTriggerType(t) + } + _, err := w.tfClient.Client.NotificationConfigurations.Update(ctx, wn.ID, tfc.NotificationConfigurationUpdateOptions{ + Name: &sn.Name, + URL: &sn.URL, + Enabled: &sn.Enabled, + Token: &sn.Token, + Triggers: triggers, + EmailAddresses: sn.EmailAddresses, + EmailUsers: sn.EmailUsers, }) if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to update notificaion %q", u.ID)) + w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to update notificaion %q", wn.ID)) return err } } @@ -228,14 +179,11 @@ func (r *WorkspaceReconciler) updateNotifications(ctx context.Context, w *worksp return nil } -func (r *WorkspaceReconciler) deleteNotifications(ctx context.Context, w *workspaceInstance, delete []tfc.NotificationConfiguration) error { - for _, d := range delete { - w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("deleting notificaion %q", d.ID)) - err := w.tfClient.Client.NotificationConfigurations.Delete(ctx, d.ID) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to delete notificaions %q", d.ID)) - return err - } +func (w *workspaceInstance) deleteNotification(ctx context.Context, notification tfc.NotificationConfiguration) error { + w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("deleting notificaion %q", notification.ID)) + if err := w.tfClient.Client.NotificationConfigurations.Delete(ctx, notification.ID); err != nil { + w.log.Error(err, "Reconcile Notifications", "msg", fmt.Sprintf("failed to delete notificaion %q", notification.ID)) + return err } return nil @@ -244,50 +192,41 @@ func (r *WorkspaceReconciler) deleteNotifications(ctx context.Context, w *worksp func (r *WorkspaceReconciler) reconcileNotifications(ctx context.Context, w *workspaceInstance) error { w.log.Info("Reconcile Notifications", "msg", "new reconciliation event") - spec, err := r.getInstanceNotifications(ctx, w) + workspaceNotifications, err := r.getWorkspaceNotifications(ctx, w) if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to get instance notificaions") + w.log.Error(err, "Reconcile Notifications", "msg", "failed to get workspace notifications") return err } - ws, err := r.getWorkspaceNotifications(ctx, w) + specNotifications, err := r.getInstanceNotifications(ctx, w) if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to get workspace notifications") + w.log.Error(err, "Reconcile Notifications", "msg", "failed to get instance notificaions") return err } - delete := getNotificationsToDelete(spec, ws) - if len(delete) > 0 { - w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("deleting %d notifications", len(delete))) - err := r.deleteNotifications(ctx, w, delete) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to delete a notificaion") - return err - } + if len(specNotifications) == 0 && len(workspaceNotifications) == 0 { + w.log.Info("Reconcile Notifications", "msg", "there are no notifications both in spec and workspace") + return nil } - create := getNotificationsToCreate(spec, ws) - if len(create) > 0 { - w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("creating %d notifications", len(create))) - err := r.createNotifications(ctx, w, create) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to create a notificaion") - return err - } - } + w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("there are %d notifications in spec", len(specNotifications))) + w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("there are %d notifications in workspace", len(workspaceNotifications))) - ws, err = r.getWorkspaceNotifications(ctx, w) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to get workspace notifications") - return err + for _, sn := range specNotifications { + if i, ok := hasNotificationItem(workspaceNotifications, sn); ok { + if err := w.updateNotification(ctx, sn, workspaceNotifications[i]); err != nil { + return err + } + workspaceNotifications = slice.RemoveFromSlice(workspaceNotifications, i) + } else { + if err := w.createNotification(ctx, sn); err != nil { + return err + } + } } - update := getNotificationsToUpdate(spec, ws) - if len(update) > 0 { - w.log.Info("Reconcile Notifications", "msg", fmt.Sprintf("updating %d notifications", len(update))) - err := r.updateNotifications(ctx, w, update) - if err != nil { - w.log.Error(err, "Reconcile Notifications", "msg", "failed to update a notificaion") + for _, wn := range workspaceNotifications { + if err := w.deleteNotification(ctx, wn); err != nil { return err } } diff --git a/controllers/workspace_controller_notifications_test.go b/controllers/workspace_controller_notifications_test.go index 6af6d1ad..644972b0 100644 --- a/controllers/workspace_controller_notifications_test.go +++ b/controllers/workspace_controller_notifications_test.go @@ -18,7 +18,7 @@ import ( appv1alpha2 "github.com/hashicorp/hcp-terraform-operator/api/v1alpha2" ) -var _ = Describe("Workspace controller", Label("Notifications"), Ordered, func() { +var _ = Describe("Workspace controller", Ordered, func() { var ( instance *appv1alpha2.Workspace namespacedName types.NamespacedName @@ -67,6 +67,7 @@ var _ = Describe("Workspace controller", Label("Notifications"), Ordered, func() }, }, Name: workspace, + Description: "Notifications", Notifications: []appv1alpha2.Notification{}, }, Status: appv1alpha2.WorkspaceStatus{}, From 14f9e5ab359ac5055c6c021a407aec105f21c324 Mon Sep 17 00:00:00 2001 From: Aleksandr Rybolovlev Date: Fri, 18 Oct 2024 10:29:30 +0200 Subject: [PATCH 3/3] Add destroy deletion policy (#489) --- .../FEATURES-489-20241017-094021.yaml | 5 ++ api/v1alpha2/module_types.go | 2 +- api/v1alpha2/workspace_types.go | 4 + api/v1alpha2/workspace_validation.go | 16 ++++ api/v1alpha2/workspace_validation_test.go | 38 +++++++++ .../crds/app.terraform.io_modules.yaml | 2 +- .../crds/app.terraform.io_workspaces.yaml | 3 + .../crd/bases/app.terraform.io_modules.yaml | 2 +- .../bases/app.terraform.io_workspaces.yaml | 3 + controllers/module_controller.go | 12 ++- .../workspace_controller_deletion_policy.go | 85 ++++++++++++++++++- ...rkspace_controller_deletion_policy_test.go | 58 ++++++++++++- 12 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 .changes/unreleased/FEATURES-489-20241017-094021.yaml diff --git a/.changes/unreleased/FEATURES-489-20241017-094021.yaml b/.changes/unreleased/FEATURES-489-20241017-094021.yaml new file mode 100644 index 00000000..d62febce --- /dev/null +++ b/.changes/unreleased/FEATURES-489-20241017-094021.yaml @@ -0,0 +1,5 @@ +kind: FEATURES +body: '`Workspace`: Add the `destroy` deletion policy. The `spec.allowDestroyPlan` must be set to `true` for the controller to execute a destroy run.' +time: 2024-10-17T09:40:21.262822+02:00 +custom: + PR: "489" diff --git a/api/v1alpha2/module_types.go b/api/v1alpha2/module_types.go index cace291d..0c29e463 100644 --- a/api/v1alpha2/module_types.go +++ b/api/v1alpha2/module_types.go @@ -141,7 +141,7 @@ type ModuleStatus struct { Run *RunStatus `json:"run,omitempty"` // Module Outputs status. Output *OutputStatus `json:"output,omitempty"` - // Workspace Destroy Run status. + // Workspace Destroy Run ID. // //+optional DestroyRunID string `json:"destroyRunID,omitempty"` diff --git a/api/v1alpha2/workspace_types.go b/api/v1alpha2/workspace_types.go index dc12f47a..cd75e4f8 100644 --- a/api/v1alpha2/workspace_types.go +++ b/api/v1alpha2/workspace_types.go @@ -674,6 +674,10 @@ type WorkspaceStatus struct { // //+optional DefaultProjectID string `json:"defaultProjectID,omitempty"` + // Workspace Destroy Run ID. + // + //+optional + DestroyRunID string `json:"destroyRunID,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1alpha2/workspace_validation.go b/api/v1alpha2/workspace_validation.go index b992c78f..b9f7cefe 100644 --- a/api/v1alpha2/workspace_validation.go +++ b/api/v1alpha2/workspace_validation.go @@ -25,6 +25,7 @@ func (w *Workspace) ValidateSpec() error { allErrs = append(allErrs, w.validateSpecProject()...) allErrs = append(allErrs, w.validateSpecTerraformVariables()...) allErrs = append(allErrs, w.validateSpecEnvironmentVariables()...) + allErrs = append(allErrs, w.validateSpecDeletionPolicy()...) if len(allErrs) == 0 { return nil @@ -568,6 +569,21 @@ func (w *Workspace) validateSpecEnvironmentVariables() field.ErrorList { return validateSpecVariables(field.NewPath("spec").Child("environmentVariables"), spec) } +func (w *Workspace) validateSpecDeletionPolicy() field.ErrorList { + allErrs := field.ErrorList{} + + f := field.NewPath("spec").Child("deletionPolicy") + + if w.Spec.DeletionPolicy == DeletionPolicyDestroy && !w.Spec.AllowDestroyPlan { + allErrs = append(allErrs, field.Required( + f, + fmt.Sprintf("'spec.allowDestroyPlan' must be set to 'true' when 'spec.deletionPolicy' is set to %q", DeletionPolicyDestroy)), + ) + } + + return allErrs +} + // TODO:Validation // // + Tags duplicate: spec.tags[] diff --git a/api/v1alpha2/workspace_validation_test.go b/api/v1alpha2/workspace_validation_test.go index c15aa817..61a38665 100644 --- a/api/v1alpha2/workspace_validation_test.go +++ b/api/v1alpha2/workspace_validation_test.go @@ -1038,3 +1038,41 @@ func TestValidateWorkspaceSpecVariables(t *testing.T) { }) } } + +func TestValidateSpecDeletionPolicy(t *testing.T) { + t.Parallel() + + successCases := map[string]Workspace{ + "DeletionPolicyDestroyAllowDestroyPlanTrue": { + Spec: WorkspaceSpec{ + AllowDestroyPlan: true, + DeletionPolicy: DeletionPolicyDestroy, + }, + }, + } + + for n, c := range successCases { + t.Run(n, func(t *testing.T) { + if errs := c.validateSpecDeletionPolicy(); len(errs) != 0 { + t.Errorf("Unexpected validation errors: %v", errs) + } + }) + } + + errorCases := map[string]Workspace{ + "DeletionPolicyDestroyAllowDestroyPlanFalse": { + Spec: WorkspaceSpec{ + AllowDestroyPlan: false, + DeletionPolicy: DeletionPolicyDestroy, + }, + }, + } + + for n, c := range errorCases { + t.Run(n, func(t *testing.T) { + if errs := c.validateSpecDeletionPolicy(); len(errs) == 0 { + t.Error("Unexpected failure, at least one error is expected") + } + }) + } +} diff --git a/charts/hcp-terraform-operator/crds/app.terraform.io_modules.yaml b/charts/hcp-terraform-operator/crds/app.terraform.io_modules.yaml index e61a7600..1f466990 100644 --- a/charts/hcp-terraform-operator/crds/app.terraform.io_modules.yaml +++ b/charts/hcp-terraform-operator/crds/app.terraform.io_modules.yaml @@ -206,7 +206,7 @@ spec: - status type: object destroyRunID: - description: Workspace Destroy Run status. + description: Workspace Destroy Run ID. type: string observedGeneration: description: Real world state generation. diff --git a/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml b/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml index b6340427..fdc91bbd 100644 --- a/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml +++ b/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml @@ -754,6 +754,9 @@ spec: defaultProjectID: description: Default organization project ID. type: string + destroyRunID: + description: Workspace Destroy Run ID. + type: string observedGeneration: description: Real world state generation. format: int64 diff --git a/config/crd/bases/app.terraform.io_modules.yaml b/config/crd/bases/app.terraform.io_modules.yaml index e494ad6d..8af35bdf 100644 --- a/config/crd/bases/app.terraform.io_modules.yaml +++ b/config/crd/bases/app.terraform.io_modules.yaml @@ -203,7 +203,7 @@ spec: - status type: object destroyRunID: - description: Workspace Destroy Run status. + description: Workspace Destroy Run ID. type: string observedGeneration: description: Real world state generation. diff --git a/config/crd/bases/app.terraform.io_workspaces.yaml b/config/crd/bases/app.terraform.io_workspaces.yaml index 1854a780..9de7b19c 100644 --- a/config/crd/bases/app.terraform.io_workspaces.yaml +++ b/config/crd/bases/app.terraform.io_workspaces.yaml @@ -751,6 +751,9 @@ spec: defaultProjectID: description: Default organization project ID. type: string + destroyRunID: + description: Workspace Destroy Run ID. + type: string observedGeneration: description: Real world state generation. format: int64 diff --git a/controllers/module_controller.go b/controllers/module_controller.go index de708bad..e1480a22 100644 --- a/controllers/module_controller.go +++ b/controllers/module_controller.go @@ -46,10 +46,16 @@ type moduleInstance struct { } var ( - runCompleteStatus = map[tfc.RunStatus]struct{}{ + runStatusComplete = map[tfc.RunStatus]struct{}{ tfc.RunApplied: {}, tfc.RunPlannedAndFinished: {}, } + + runStatusUnsuccessful = map[tfc.RunStatus]struct{}{ + tfc.RunCanceled: {}, + tfc.RunDiscarded: {}, + tfc.RunErrored: {}, + } ) // +kubebuilder:rbac:groups=app.terraform.io,resources=modules,verbs=get;list;watch;create;update;patch;delete @@ -268,7 +274,7 @@ func (r *ModuleReconciler) deleteModule(ctx context.Context, m *moduleInstance) } if cr.IsDestroy { m.log.Info("Delete Module", "msg", fmt.Sprintf("current run %s is destroy", cr.ID)) - if _, ok := runCompleteStatus[cr.Status]; ok { + if _, ok := runStatusComplete[cr.Status]; ok { m.log.Info("Delete Module", "msg", "current destroy run finished") return r.removeFinalizer(ctx, m) } @@ -302,7 +308,7 @@ func (r *ModuleReconciler) deleteModule(ctx context.Context, m *moduleInstance) } m.log.Info("Reconcile Run", "msg", fmt.Sprintf("successfully got destroy run status: %s", run.Status)) - if _, ok := runCompleteStatus[run.Status]; ok { + if _, ok := runStatusComplete[run.Status]; ok { m.log.Info("Delete Module", "msg", "destroy run finished") return r.removeFinalizer(ctx, m) } diff --git a/controllers/workspace_controller_deletion_policy.go b/controllers/workspace_controller_deletion_policy.go index 92b1cf63..dd23992a 100644 --- a/controllers/workspace_controller_deletion_policy.go +++ b/controllers/workspace_controller_deletion_policy.go @@ -43,8 +43,70 @@ func (r *WorkspaceReconciler) deleteWorkspace(ctx context.Context, w *workspaceI w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("workspace ID %s has been deleted, remove finalizer", w.instance.Status.WorkspaceID)) return r.removeFinalizer(ctx, w) case appv1alpha2.DeletionPolicyDestroy: - // TODO: Implement the destroy logic - return nil + if w.instance.Status.DestroyRunID == "" { + workspace, err := w.tfClient.Client.Workspaces.ReadByID(ctx, w.instance.Status.WorkspaceID) + if err != nil { + return r.handleWorkspaceErrorNotFound(ctx, w, err) + } + if workspace.CurrentRun == nil { + w.log.Info("Reconcile Workspace", "msg", "Workspace does not have runs, skipping destroy run, and remove finalizer") + if err := w.tfClient.Client.Workspaces.DeleteByID(ctx, w.instance.Status.WorkspaceID); err != nil { + return r.handleWorkspaceErrorNotFound(ctx, w, err) + } + + w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("workspace ID %s has been deleted, remove finalizer", w.instance.Status.WorkspaceID)) + return r.removeFinalizer(ctx, w) + } + w.log.Info("Destroy Run", "msg", "destroy on deletion, create a new destroy run") + run, err := w.tfClient.Client.Runs.Create(ctx, tfc.RunCreateOptions{ + IsDestroy: tfc.Bool(true), + Message: tfc.String(runMessage), + Workspace: &tfc.Workspace{ + ID: w.instance.Status.WorkspaceID, + }, + }) + if err != nil { + w.log.Error(err, "Destroy Run", "msg", "failed to create a new destroy run") + return err + } + w.log.Info("Destroy Run", "msg", fmt.Sprintf("successfully created a new destroy run: %s", run.ID)) + + w.instance.Status.DestroyRunID = run.ID + w.updateWorkspaceStatusRun(run) + return r.Status().Update(ctx, &w.instance) + } + + w.log.Info("Destroy Run", "msg", fmt.Sprintf("get destroy run %s", w.instance.Status.DestroyRunID)) + run, err := w.tfClient.Client.Runs.Read(ctx, w.instance.Status.DestroyRunID) + if err != nil { + if err == tfc.ErrResourceNotFound { + w.log.Info("Reconcile Workspace", "msg", "Destroy run was not found, check if the workspace exists") + if _, err := w.tfClient.Client.Workspaces.ReadByID(ctx, w.instance.Status.WorkspaceID); err != nil { + return r.handleWorkspaceErrorNotFound(ctx, w, err) + } + } + w.log.Info("Destroy Run", "msg", fmt.Sprintf("failed to get destroy run: %s", w.instance.Status.DestroyRunID)) + return err + } + + if _, ok := runStatusComplete[run.Status]; ok { + w.log.Info("Destroy Run", "msg", fmt.Sprintf("current destroy run %s is finished", run.ID)) + if err := w.tfClient.Client.Workspaces.DeleteByID(ctx, w.instance.Status.WorkspaceID); err != nil { + return r.handleWorkspaceErrorNotFound(ctx, w, err) + } + + w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("workspace ID %s has been deleted, remove finalizer", w.instance.Status.WorkspaceID)) + return r.removeFinalizer(ctx, w) + } + + if _, ok := runStatusUnsuccessful[run.Status]; ok { + w.log.Info("Destroy Run", "msg", fmt.Sprintf("destroy run %s is unsuccessful: %s", run.ID, run.Status)) + return nil + } + w.log.Info("Destroy Run", "msg", fmt.Sprintf("destroy run %s is not finished", run.ID)) + + w.updateWorkspaceStatusRun(run) + return r.Status().Update(ctx, &w.instance) case appv1alpha2.DeletionPolicyForce: err := w.tfClient.Client.Workspaces.DeleteByID(ctx, w.instance.Status.WorkspaceID) if err != nil { @@ -63,3 +125,22 @@ func (r *WorkspaceReconciler) deleteWorkspace(ctx context.Context, w *workspaceI return nil } + +func (r *WorkspaceReconciler) handleWorkspaceErrorNotFound(ctx context.Context, w *workspaceInstance, err error) error { + if err == tfc.ErrResourceNotFound { + w.log.Info("Reconcile Workspace", "msg", "Workspace was not found, remove finalizer") + return r.removeFinalizer(ctx, w) + } + w.log.Error(err, "Reconcile Workspace", "msg", fmt.Sprintf("failed to handle Workspace ID %s, retry later", w.instance.Status.WorkspaceID)) + r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to handle Workspace ID %s, retry later", w.instance.Status.WorkspaceID) + return err +} + +func (w *workspaceInstance) updateWorkspaceStatusRun(run *tfc.Run) { + if w.instance.Status.Run == nil { + w.instance.Status.Run = &appv1alpha2.RunStatus{} + } + w.instance.Status.Run.ID = run.ID + w.instance.Status.Run.Status = string(run.Status) + w.instance.Status.Run.ConfigurationVersion = run.ConfigurationVersion.ID +} diff --git a/controllers/workspace_controller_deletion_policy_test.go b/controllers/workspace_controller_deletion_policy_test.go index 9e621e8d..5649fb7a 100644 --- a/controllers/workspace_controller_deletion_policy_test.go +++ b/controllers/workspace_controller_deletion_policy_test.go @@ -136,8 +136,64 @@ var _ = Describe("Workspace controller", Ordered, func() { }).Should(BeTrue()) }) It("can destroy delete a workspace", func() { - // Not yet implemented + if cloudEndpoint != tfcDefaultAddress { + Skip("Does not run against TFC, skip this test") + } + instance.Spec.AllowDestroyPlan = true instance.Spec.DeletionPolicy = appv1alpha2.DeletionPolicyDestroy + createWorkspace(instance) + workspaceID := instance.Status.WorkspaceID + + cv := createAndUploadConfigurationVersion(instance, "hoi") + Eventually(func() bool { + listOpts := tfc.ListOptions{ + PageNumber: 1, + PageSize: maxPageSize, + } + for listOpts.PageNumber != 0 { + runs, err := tfClient.Runs.List(ctx, workspaceID, &tfc.RunListOptions{ + ListOptions: listOpts, + }) + Expect(err).To(Succeed()) + for _, r := range runs.Items { + if r.ConfigurationVersion.ID == cv.ID { + return r.Status == tfc.RunApplied + } + } + listOpts.PageNumber = runs.NextPage + } + return false + }).Should(BeTrue()) + + Expect(k8sClient.Delete(ctx, instance)).To(Succeed()) + + var destroyRunID string + Eventually(func() bool { + ws, err := tfClient.Workspaces.ReadByID(ctx, workspaceID) + Expect(err).To(Succeed()) + Expect(ws).ToNot(BeNil()) + Expect(ws.CurrentRun).ToNot(BeNil()) + run, err := tfClient.Runs.Read(ctx, ws.CurrentRun.ID) + Expect(err).To(Succeed()) + Expect(run).ToNot(BeNil()) + destroyRunID = run.ID + + return run.IsDestroy + }).Should(BeTrue()) + + Eventually(func() bool { + run, err := tfClient.Runs.Read(ctx, destroyRunID) + if err == tfc.ErrResourceNotFound || run.Status == tfc.RunApplied { + return true + } + + return false + }).Should(BeTrue()) + + Eventually(func() bool { + _, err := tfClient.Workspaces.ReadByID(ctx, workspaceID) + return err == tfc.ErrResourceNotFound + }).Should(BeTrue()) }) It("can force delete a workspace", func() { instance.Spec.DeletionPolicy = appv1alpha2.DeletionPolicyForce