feat(teams): add members to team status (#794)

* feat(teams) Add members to team status (#723) * Automatic generation of CRD API Docs * Automatic application of license header * feat(teams) Unit tests for members in team (#723) * feat(teams) Team controller is running in team meme=bership tests (#723) * Automatic application of license header * Automatic generation of CRD API Docs * feat(teams) Update unit test and update (#723) * feat(teams) Add team controller responsibility to team membership updater (#723) * Automatic application of license header * feat(teams) Restore namespace selectors (#723) --------- Co-authored-by: Cloud Operator <[email protected]> Co-authored-by: License Bot <[email protected]>
cloudoperators · Dec 17, 2024 · 06181e1 · 06181e1
1 parent 0f46faf
commit 06181e1
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 9 deletions.
diff --git a/charts/manager/crds/greenhouse.sap_teams.yaml b/charts/manager/crds/greenhouse.sap_teams.yaml
@@ -65,6 +65,29 @@ spec:
           status:
             description: TeamStatus defines the observed state of Team
             properties:
+              members:
+                items:
+                  description: User specifies a human person.
+                  properties:
+                    email:
+                      description: Email of the user.
+                      type: string
+                    firstName:
+                      description: FirstName of the user.
+                      type: string
+                    id:
+                      description: ID is the unique identifier of the user.
+                      type: string
+                    lastName:
+                      description: LastName of the user.
+                      type: string
+                  required:
+                  - email
+                  - firstName
+                  - id
+                  - lastName
+                  type: object
+                type: array
               statusConditions:
                 description: |-
                   A StatusConditions contains a list of conditions.
@@ -105,6 +128,7 @@ spec:
                     x-kubernetes-list-type: map
                 type: object
             required:
+            - members
             - statusConditions
             type: object
         type: object

diff --git a/charts/manager/templates/manager-role.yaml b/charts/manager/templates/manager-role.yaml
@@ -8,10 +8,10 @@ rules:
   - ""
   resources:
   - events
-  - namespaces
+  - secrets
+  - serviceaccounts
   verbs:
   - create
-  - delete
   - get
   - list
   - patch
@@ -20,10 +20,10 @@ rules:
 - apiGroups:
   - ""
   resources:
-  - secrets
-  - serviceaccounts
+  - namespaces
   verbs:
   - create
+  - delete
   - get
   - list
   - patch

diff --git a/docs/reference/api/index.html b/docs/reference/api/index.html
@@ -3456,6 +3456,18 @@ <h3 id="greenhouse.sap/v1alpha1.TeamStatus">TeamStatus
 <td>
 </td>
 </tr>
+<tr>
+<td>
+<code>members</code><br>
+<em>
+<a href="#greenhouse.sap/v1alpha1.User">
+[]User
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
 </tbody>
 </table>
 </div>
@@ -3520,7 +3532,8 @@ <h3 id="greenhouse.sap/v1alpha1.User">User
 </h3>
 <p>
 (<em>Appears on:</em>
-<a href="#greenhouse.sap/v1alpha1.TeamMembershipSpec">TeamMembershipSpec</a>)
+<a href="#greenhouse.sap/v1alpha1.TeamMembershipSpec">TeamMembershipSpec</a>, 
+<a href="#greenhouse.sap/v1alpha1.TeamStatus">TeamStatus</a>)
 </p>
 <p>User specifies a human person.</p>
 <div class="md-typeset__scrollwrap">

diff --git a/docs/reference/api/openapi.yaml b/docs/reference/api/openapi.yaml
@@ -1107,6 +1107,29 @@ components:
         status:
           description: TeamStatus defines the observed state of Team
           properties:
+            members:
+              items:
+                description: User specifies a human person.
+                properties:
+                  email:
+                    description: Email of the user.
+                    type: string
+                  firstName:
+                    description: FirstName of the user.
+                    type: string
+                  id:
+                    description: ID is the unique identifier of the user.
+                    type: string
+                  lastName:
+                    description: LastName of the user.
+                    type: string
+                required:
+                  - email
+                  - firstName
+                  - id
+                  - lastName
+                type: object
+              type: array
             statusConditions:
               description: A StatusConditions contains a list of conditions.\nOnly one condition of a given type may exist in the list.
               properties:
@@ -1141,6 +1164,7 @@ components:
                   x-kubernetes-list-type: map
               type: object
           required:
+            - members
             - statusConditions
           type: object
       type: object

diff --git a/pkg/apis/greenhouse/v1alpha1/team_types.go b/pkg/apis/greenhouse/v1alpha1/team_types.go
@@ -20,6 +20,7 @@ type TeamSpec struct {
 // TeamStatus defines the observed state of Team
 type TeamStatus struct {
 	StatusConditions StatusConditions `json:"statusConditions"`
+	Members          []User           `json:"members"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/pkg/apis/greenhouse/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/greenhouse/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/controllers/teammembership/teammembership_updater_controller.go b/pkg/controllers/teammembership/teammembership_updater_controller.go
@@ -99,7 +99,7 @@ func listTeamsAsReconcileRequests(ctx context.Context, c client.Client, listOpts
 }
 
 func (r *TeamMembershipUpdaterController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	return lifecycle.Reconcile(ctx, r.Client, req.NamespacedName, &greenhousev1alpha1.Team{}, r, nil) // status function is nil because it updates the other entity inside.
+	return lifecycle.Reconcile(ctx, r.Client, req.NamespacedName, &greenhousev1alpha1.Team{}, r, nil)
 }
 
 func (r *TeamMembershipUpdaterController) EnsureDeleted(_ context.Context, _ lifecycle.RuntimeObject) (ctrl.Result, lifecycle.ReconcileResult, error) {
@@ -187,6 +187,7 @@ func (r *TeamMembershipUpdaterController) EnsureCreated(ctx context.Context, obj
 		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
+	team.Status.Members = users
 	teamMembershipStatus.SetConditions(membersValidCondition, greenhousev1alpha1.TrueCondition(greenhousev1alpha1.SCIMAccessReadyCondition, "", ""))
 
 	membersCountMetric.With(prometheus.Labels{

diff --git a/pkg/controllers/teammembership/teammembership_updater_controller_test.go b/pkg/controllers/teammembership/teammembership_updater_controller_test.go
@@ -69,14 +69,20 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 				g.Expect(readyCondition.Type).To(Equal(greenhousev1alpha1.ReadyCondition))
 				g.Expect(readyCondition.Status).To(Equal(metav1.ConditionTrue))
 			}).Should(Succeed(), "TeamMembership should be reconciled")
+
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(team), team)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting Team")
+				g.Expect(team.Status.Members).To(HaveLen(2), "the Team should have exactly two Members")
+			}).Should(Succeed(), "Team should have the team members")
 		})
 
 		It("should update existing TM without users", func() {
 			By("creating a test TeamMembership")
 			createTeamMembershipForFirstTeam(nil)
 
 			By("creating a test Team")
-			createFirstTeam(validIdpGroupName)
+			firstTeam := createFirstTeam(validIdpGroupName)
 
 			By("ensuring the TeamMembership has been reconciled")
 			Eventually(func(g Gomega) {
@@ -96,6 +102,13 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 				g.Expect(readyCondition.Type).To(Equal(greenhousev1alpha1.ReadyCondition))
 				g.Expect(readyCondition.Status).To(Equal(metav1.ConditionTrue))
 			}).Should(Succeed(), "the TeamMembership should be reconciled")
+
+			By("ensuring that the Team has been updated")
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(firstTeam), firstTeam)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting Team")
+				g.Expect(firstTeam.Status.Members).To(HaveLen(2), "the TeamMembership should have exactly two Members")
+			}).Should(Succeed(), "Team should have the team members")
 		})
 
 		It("should update existing TM with users", func() {
@@ -110,7 +123,7 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 			})
 
 			By("creating a test Team")
-			createFirstTeam(validIdpGroupName)
+			firstTeam := createFirstTeam(validIdpGroupName)
 
 			By("ensuring the TeamMembership has been reconciled")
 			Eventually(func(g Gomega) {
@@ -121,6 +134,13 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 				g.Expect(teamMemberships.Items[0].Spec.Members).To(HaveLen(2), "the TeamMembership should have exactly two Members")
 				g.Expect(teamMemberships.Items[0].Status.LastChangedTime).ToNot(BeNil(), "TeamMembership status should have updated LastChangedTime")
 			}).Should(Succeed(), "the TeamMembership should be reconciled")
+
+			By("ensuring that the Team has been updated")
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(firstTeam), firstTeam)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting Team")
+				g.Expect(firstTeam.Status.Members).To(HaveLen(2), "the TeamMembership should have exactly two Members")
+			}).Should(Succeed(), "Team should have the team members")
 		})
 
 		It("should update multiple TMs", func() {
@@ -172,6 +192,20 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 				g.Expect(teamMemberships.Items[0].OwnerReferences).To(ContainElement(firstOwnerRef), "first TeamMembership should have set first team as owner reference")
 				g.Expect(teamMemberships.Items[1].OwnerReferences).To(ContainElement(secondOwnerRef), "second TeamMembership should have set second team as owner reference")
 			}).Should(Succeed(), "both TeamMemberships should be reconciled")
+
+			By("ensuring that the first Team has been updated")
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(firstTeam), firstTeam)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting Team")
+				g.Expect(firstTeam.Status.Members).To(HaveLen(2), "the first team should have exactly two Members")
+			}).Should(Succeed(), "First team should have the team members")
+
+			By("ensuring that the second Team has been updated")
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(secondTeam), secondTeam)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting Team")
+				g.Expect(secondTeam.Status.Members).To(HaveLen(3), "the second team should have exactly three Members")
+			}).Should(Succeed(), "Second team should have the team members")
 		})
 
 		It("should do nothing if Team has no mappedIdpGroup", func() {
@@ -367,7 +401,7 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 			createTeamMembershipForFirstTeam(originalUsers)
 
 			By("creating test Team with valid idp group")
-			createFirstTeam(validIdpGroupName)
+			firstTeam := createFirstTeam(validIdpGroupName)
 
 			expectedUser1 := greenhousev1alpha1.User{
 				ID:        "I12345",
@@ -393,6 +427,13 @@ var _ = Describe("TeammembershipUpdaterController", Ordered, func() {
 				g.Expect(teamMembership.Spec.Members).To(ContainElement(expectedUser1), "TeamMembership users should contain first expected user")
 				g.Expect(teamMembership.Spec.Members).To(ContainElement(expectedUser2), "TeamMembership users should contain second expected user")
 			}).Should(Succeed(), "TeamMembership should have been reconciled")
+
+			Eventually(func(g Gomega) {
+				err := setup.Get(test.Ctx, client.ObjectKeyFromObject(firstTeam), firstTeam)
+				g.Expect(err).ShouldNot(HaveOccurred(), "unexpected error getting firstTeam")
+				g.Expect(firstTeam.Status.Members).To(HaveLen(2), "firstTeam should have two users")
+				g.Expect(firstTeam.Status.Members).To(BeEquivalentTo([]greenhousev1alpha1.User{expectedUser1, expectedUser2}), "firstTeam should contains users")
+			}).Should(Succeed(), "the team status should be updated")
 		})
 
 		It("should delete the team after all", func() {

diff --git a/test/e2e/controllers.go b/test/e2e/controllers.go
@@ -36,6 +36,7 @@ var knownControllers = map[string]func(controllerName string, mgr ctrl.Manager)
 	"organizationTeamRoleSeeder": (&organizationcontrollers.TeamRoleSeederReconciler{}).SetupWithManager,
 
 	// Team controllers.
+	"teamController":  (&teamcontrollers.TeamReconciler{}).SetupWithManager,
 	"teamPropagation": (&teamcontrollers.TeamPropagationReconciler{}).SetupWithManager,
 
 	// TeamMembership controllers.