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.

.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"

.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"

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

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

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

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}
+			}
 		}
 	}
 

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")

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())
 }