From 601ef7dd9a4bf47d06f8a1fa957c53a7d711efd4 Mon Sep 17 00:00:00 2001 From: Aleksandr Rybolovlev Date: Wed, 25 Sep 2024 11:52:44 +0200 Subject: [PATCH] Update Workspace controller default project ID reconciliation logic (#481) * Update Workspace controller reconciliation logic to avoid pulling default project ID on every reconciliation when workspace gets updated. --- .../ENHANCEMENTS-481-20240830-094816.yaml | 5 +++ .../unreleased/NOTES-481-20240830-094502.yaml | 5 +++ api/v1alpha2/workspace_types.go | 4 ++ .../crds/app.terraform.io_workspaces.yaml | 3 ++ .../bases/app.terraform.io_workspaces.yaml | 3 ++ controllers/workspace_controller.go | 44 +++++++++++-------- controllers/workspace_controller_projects.go | 6 +-- .../workspace_controller_projects_test.go | 17 +++---- 8 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 .changes/unreleased/ENHANCEMENTS-481-20240830-094816.yaml create mode 100644 .changes/unreleased/NOTES-481-20240830-094502.yaml diff --git a/.changes/unreleased/ENHANCEMENTS-481-20240830-094816.yaml b/.changes/unreleased/ENHANCEMENTS-481-20240830-094816.yaml new file mode 100644 index 00000000..cc3a4825 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-481-20240830-094816.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: '`Workspace`: Update the default project ID reconciliation logic to avoid making an API call each time a workspace object is updated.' +time: 2024-08-30T09:48:16.140808+02:00 +custom: + PR: "481" diff --git a/.changes/unreleased/NOTES-481-20240830-094502.yaml b/.changes/unreleased/NOTES-481-20240830-094502.yaml new file mode 100644 index 00000000..529e72d3 --- /dev/null +++ b/.changes/unreleased/NOTES-481-20240830-094502.yaml @@ -0,0 +1,5 @@ +kind: NOTES +body: The `Workspace` CRD has been changed. Please follow the Helm chart instructions on how to upgrade it. +time: 2024-08-30T09:45:02.393365+02:00 +custom: + PR: "481" diff --git a/api/v1alpha2/workspace_types.go b/api/v1alpha2/workspace_types.go index e4f488b6..d577252d 100644 --- a/api/v1alpha2/workspace_types.go +++ b/api/v1alpha2/workspace_types.go @@ -641,6 +641,10 @@ type WorkspaceStatus struct { // //+optional Variables []VariableStatus `json:"variables,omitempty"` + // Default organization project ID. + // + //+optional + DefaultProjectID string `json:"defaultProjectID,omitempty"` } //+kubebuilder:object:root=true 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 8fbf379d..f59586fd 100644 --- a/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml +++ b/charts/hcp-terraform-operator/crds/app.terraform.io_workspaces.yaml @@ -736,6 +736,9 @@ spec: status: description: WorkspaceStatus defines the observed state of Workspace. properties: + defaultProjectID: + description: Default organization project ID. + type: string observedGeneration: description: Real world state generation. format: int64 diff --git a/config/crd/bases/app.terraform.io_workspaces.yaml b/config/crd/bases/app.terraform.io_workspaces.yaml index fa028c4d..2a376864 100644 --- a/config/crd/bases/app.terraform.io_workspaces.yaml +++ b/config/crd/bases/app.terraform.io_workspaces.yaml @@ -733,6 +733,9 @@ spec: status: description: WorkspaceStatus defines the observed state of Workspace. properties: + defaultProjectID: + description: Default organization project ID. + type: string observedGeneration: description: Real world state generation. format: int64 diff --git a/controllers/workspace_controller.go b/controllers/workspace_controller.go index 6f3542f8..8695d853 100644 --- a/controllers/workspace_controller.go +++ b/controllers/workspace_controller.go @@ -283,8 +283,15 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI options.GlobalRemoteState = tfc.Bool(spec.RemoteStateSharing.AllWorkspaces) } + org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization) + if err != nil { + w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization") + r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization") + return nil, err + } + if spec.Project != nil { - prjID, err := r.getProjectID(ctx, w) + prjID, err := w.getProjectID(ctx) if err != nil { w.log.Error(err, "Reconcile Workspace", "msg", "failed to get project ID") r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get project ID") @@ -300,6 +307,9 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI } patch := client.MergeFrom(w.instance.DeepCopy()) + if org != nil && org.DefaultProject != nil { + w.instance.Status.DefaultProjectID = org.DefaultProject.ID + } w.instance.Status.WorkspaceID = workspace.ID if err = r.Status().Patch(ctx, &w.instance, patch); err != nil { w.log.Error(err, "Reconcile Workspace", "msg", "failed to update status with workspace ID") @@ -397,7 +407,7 @@ func (r *WorkspaceReconciler) updateWorkspace(ctx context.Context, w *workspaceI } if spec.Project != nil { - prjID, err := r.getProjectID(ctx, w) + prjID, err := w.getProjectID(ctx) if err != nil { w.log.Error(err, "Reconcile Workspace", "msg", "failed to get project ID") r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get project ID") @@ -406,22 +416,20 @@ func (r *WorkspaceReconciler) updateWorkspace(ctx context.Context, w *workspaceI w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("project ID %s will be used", prjID)) updateOptions.Project = &tfc.Project{ID: prjID} } else { - // Setting up `Project` to nil(tfc.WorkspaceUpdateOptions{Project: nil}) won't move the workspace to the default project after the update. - // TODO: - // - The default project can be renamed, but cannot be deleted. - // We should do this API call once on a workspace creation and keep the default project ID in the status for future reference. - // We could validate whether or not the default project ID in the status is empty or not during the update stage. - // If the default project ID in the status is empty, then we make an API call to fill in this gap. - // Ideally, it will never happen but can during the TFE upgrade. - org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization) - if err != nil { - w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization") - r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization") - return nil, err - } - if org.DefaultProject != nil { - w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", org.DefaultProject.ID)) - updateOptions.Project = &tfc.Project{ID: org.DefaultProject.ID} + if w.instance.Status.DefaultProjectID != "" { + updateOptions.Project = &tfc.Project{ID: w.instance.Status.DefaultProjectID} + w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", w.instance.Status.DefaultProjectID)) + } else { + org, err := w.tfClient.Client.Organizations.Read(ctx, w.instance.Spec.Organization) + if err != nil { + w.log.Error(err, "Reconcile Workspace", "msg", "failed to get organization") + r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get organization") + return nil, err + } + if org != nil && org.DefaultProject != nil { + w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("default project ID %s will be used", org.DefaultProject.ID)) + updateOptions.Project = &tfc.Project{ID: org.DefaultProject.ID} + } } } diff --git a/controllers/workspace_controller_projects.go b/controllers/workspace_controller_projects.go index 7f4edca9..075b1b6d 100644 --- a/controllers/workspace_controller_projects.go +++ b/controllers/workspace_controller_projects.go @@ -10,7 +10,7 @@ import ( tfc "github.com/hashicorp/go-tfe" ) -func (r *WorkspaceReconciler) getProjectIDByName(ctx context.Context, w *workspaceInstance) (string, error) { +func (w *workspaceInstance) getProjectIDByName(ctx context.Context) (string, error) { projectName := w.instance.Spec.Project.Name listOpts := &tfc.ProjectListOptions{ @@ -38,7 +38,7 @@ func (r *WorkspaceReconciler) getProjectIDByName(ctx context.Context, w *workspa return "", fmt.Errorf("project ID not found for project name %q", projectName) } -func (r *WorkspaceReconciler) getProjectID(ctx context.Context, w *workspaceInstance) (string, error) { +func (w *workspaceInstance) getProjectID(ctx context.Context) (string, error) { specProject := w.instance.Spec.Project if specProject == nil { @@ -47,7 +47,7 @@ func (r *WorkspaceReconciler) getProjectID(ctx context.Context, w *workspaceInst if specProject.Name != "" { w.log.Info("Reconcile Project", "msg", "getting project ID by name") - return r.getProjectIDByName(ctx, w) + return w.getProjectIDByName(ctx) } w.log.Info("Reconcile Project", "msg", "getting project ID from the spec.Project.ID") diff --git a/controllers/workspace_controller_projects_test.go b/controllers/workspace_controller_projects_test.go index 4cd7ff8f..142fe2f6 100644 --- a/controllers/workspace_controller_projects_test.go +++ b/controllers/workspace_controller_projects_test.go @@ -78,7 +78,7 @@ var _ = Describe("Workspace controller", Ordered, func() { }) Context("Project", func() { - It("can handle project by name", func() { + It("can be handled by name", func() { instance.Spec.Project = &appv1alpha2.WorkspaceProject{Name: projectName} // Create a new Kubernetes workspace object and wait until the controller finishes the reconciliation createWorkspace(instance) @@ -103,7 +103,7 @@ var _ = Describe("Workspace controller", Ordered, func() { isReconciledDefaultProject(instance) }) - It("can handle project by id", func() { + It("can be handled by ID", func() { instance.Spec.Project = &appv1alpha2.WorkspaceProject{ID: projectID} // Create a new Kubernetes workspace object and wait until the controller finishes the reconciliation createWorkspace(instance) @@ -145,8 +145,8 @@ func isReconciledProjectByID(instance *appv1alpha2.Workspace) { Eventually(func() bool { Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed()) ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID) - Expect(ws).ShouldNot(BeNil()) Expect(err).Should(Succeed()) + Expect(ws).ShouldNot(BeNil()) return ws.Project.ID == instance.Spec.Project.ID }).Should(BeTrue()) } @@ -157,11 +157,11 @@ func isReconciledProjectByName(instance *appv1alpha2.Workspace) { Eventually(func() bool { Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed()) ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID) - Expect(ws).ShouldNot(BeNil()) Expect(err).Should(Succeed()) + Expect(ws).ShouldNot(BeNil()) prj, err := tfClient.Projects.Read(ctx, ws.Project.ID) - Expect(prj).ShouldNot(BeNil()) Expect(err).Should(Succeed()) + Expect(prj).ShouldNot(BeNil()) return prj.Name == instance.Spec.Project.Name }).Should(BeTrue()) } @@ -172,11 +172,12 @@ func isReconciledDefaultProject(instance *appv1alpha2.Workspace) { Eventually(func() bool { Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed()) ws, err := tfClient.Workspaces.ReadByID(ctx, instance.Status.WorkspaceID) - Expect(ws).ShouldNot(BeNil()) Expect(err).Should(Succeed()) + Expect(ws).ShouldNot(BeNil()) org, err := tfClient.Organizations.Read(ctx, instance.Spec.Organization) - Expect(org).ShouldNot(BeNil()) Expect(err).Should(Succeed()) - return ws.Project.ID == org.DefaultProject.ID + Expect(org).ShouldNot(BeNil()) + Expect(org.DefaultProject).ShouldNot(BeNil()) + return org.DefaultProject.ID == ws.Project.ID && org.DefaultProject.ID == instance.Status.DefaultProjectID }).Should(BeTrue()) }