fix: set required policy version to support conditions (abcxyz#25)

sqin2019 · May 25, 2023 · 050061d · 050061d
1 parent 761d273
commit 050061d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/pkg/handler/iam_handler.go b/pkg/handler/iam_handler.go
@@ -116,7 +116,18 @@ func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolic
 	var np *iampb.Policy
 	if err := retry.Do(ctx, h.retry, func(ctx context.Context) error {
 		// Get current IAM policy.
-		cp, err := iamC.GetIamPolicy(ctx, &iampb.GetIamPolicyRequest{Resource: p.Resource})
+		getIAMPolicyRequest := &iampb.GetIamPolicyRequest{
+			Resource: p.Resource,
+			// Set required policy version to 3 to support conditional IAM bindings
+			// in the requested policy.
+			// Note that if the requested policy does not contain conditional IAM
+			// bindings it will return the policy as is, which is version 1.
+			// See details here: https://cloud.google.com/iam/docs/policies#specifying-version-get
+			Options: &iampb.GetPolicyOptions{
+				RequestedPolicyVersion: 3,
+			},
+		}
+		cp, err := iamC.GetIamPolicy(ctx, getIAMPolicyRequest)
 		if err != nil {
 			return fmt.Errorf("failed to get IAM policy: %w", err)
 		}
@@ -125,11 +136,11 @@ func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolic
 		updatePolicy(cp, p.Bindings, expiry)
 
 		// Set the new policy.
-		setIamPolicyRequest := &iampb.SetIamPolicyRequest{
+		setIAMPolicyRequest := &iampb.SetIamPolicyRequest{
 			Resource: p.Resource,
 			Policy:   cp,
 		}
-		np, err = iamC.SetIamPolicy(ctx, setIamPolicyRequest)
+		np, err = iamC.SetIamPolicy(ctx, setIAMPolicyRequest)
 		// Retry when set IAM policy fail.
 		// TODO(#8): Look for specific errors to retry.
 		if err != nil {
@@ -190,6 +201,10 @@ func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) {
 		sort.Strings(newBinding.Members)
 		p.Bindings = append(p.Bindings, newBinding)
 	}
+
+	// Set policy version to 3 to support conditional IAM bindings.
+	// See details here: https://cloud.google.com/iam/docs/policies#specifying-version-set
+	p.Version = 3
 }
 
 func toBindingsMap(bs []*v1alpha1.Binding) map[string]map[string]struct{} {

diff --git a/pkg/handler/iam_handler_test.go b/pkg/handler/iam_handler_test.go
@@ -121,6 +121,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 				{
@@ -138,6 +139,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 				{
@@ -155,6 +157,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -172,6 +175,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 			wantFoldersPolicy: &iampb.Policy{
 				Bindings: []*iampb.Binding{
@@ -186,6 +190,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 			wantProjectsPolicy: &iampb.Policy{
 				Bindings: []*iampb.Binding{
@@ -200,6 +205,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 		},
 		{
@@ -293,6 +299,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -320,6 +327,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 			wantFoldersPolicy:  &iampb.Policy{},
 			wantProjectsPolicy: &iampb.Policy{},
@@ -386,6 +394,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -409,6 +418,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 			wantProjectsPolicy: &iampb.Policy{},
 		},
@@ -482,6 +492,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -510,6 +521,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 		},
 		{
@@ -571,6 +583,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -590,6 +603,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 		},
 		{
@@ -650,6 +664,7 @@ func TestDo(t *testing.T) {
 								},
 							},
 						},
+						Version: 3,
 					},
 				},
 			},
@@ -668,6 +683,7 @@ func TestDo(t *testing.T) {
 						},
 					},
 				},
+				Version: 3,
 			},
 			wantProjectsPolicy: &iampb.Policy{},
 		},