From b03941eb22018c461c083fd063000ff3bffef5d1 Mon Sep 17 00:00:00 2001 From: Rauf Guliyev Date: Mon, 20 Jul 2026 18:38:29 +0000 Subject: [PATCH] Render GCE project IAM as additive members The API target adds a principal without owning the full role binding, but the Terraform target emitted an authoritative binding that could remove unrelated principals. Use google_project_iam_member so both targets have the same additive semantics. --- .../many-addons-gce/kubernetes.tf | 8 ++-- .../minimal_gce_dns-none/kubernetes.tf | 8 ++-- .../fi/cloudup/gcetasks/projectiambinding.go | 18 +++++---- .../gcetasks/projectiambinding_test.go | 40 +++++++++++++++++++ 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/tests/integration/update_cluster/many-addons-gce/kubernetes.tf b/tests/integration/update_cluster/many-addons-gce/kubernetes.tf index c2dc0a7905bec..cc92c0ad67c1d 100644 --- a/tests/integration/update_cluster/many-addons-gce/kubernetes.tf +++ b/tests/integration/update_cluster/many-addons-gce/kubernetes.tf @@ -579,14 +579,14 @@ resource "google_compute_subnetwork" "us-test1-minimal-example-com" { stack_type = "IPV4_ONLY" } -resource "google_project_iam_binding" "serviceaccount-control-plane" { - members = [format("serviceAccount:%s", google_service_account.control-plane.email)] +resource "google_project_iam_member" "serviceaccount-control-plane" { + member = format("serviceAccount:%s", google_service_account.control-plane.email) project = "testproject" role = "roles/container.serviceAgent" } -resource "google_project_iam_binding" "serviceaccount-nodes" { - members = [format("serviceAccount:%s", google_service_account.node.email)] +resource "google_project_iam_member" "serviceaccount-nodes" { + member = format("serviceAccount:%s", google_service_account.node.email) project = "testproject" role = "roles/compute.viewer" } diff --git a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf index cb15a40223953..1871d0fef16e8 100644 --- a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf @@ -669,14 +669,14 @@ resource "google_compute_subnetwork" "us-test1-minimal-gce-example-com" { stack_type = "IPV4_ONLY" } -resource "google_project_iam_binding" "serviceaccount-control-plane" { - members = [format("serviceAccount:%s", google_service_account.control-plane.email)] +resource "google_project_iam_member" "serviceaccount-control-plane" { + member = format("serviceAccount:%s", google_service_account.control-plane.email) project = "testproject" role = "roles/container.serviceAgent" } -resource "google_project_iam_binding" "serviceaccount-nodes" { - members = [format("serviceAccount:%s", google_service_account.node.email)] +resource "google_project_iam_member" "serviceaccount-nodes" { + member = format("serviceAccount:%s", google_service_account.node.email) project = "testproject" role = "roles/compute.viewer" } diff --git a/upup/pkg/fi/cloudup/gcetasks/projectiambinding.go b/upup/pkg/fi/cloudup/gcetasks/projectiambinding.go index 5b0077a2f96e3..eb1c52613e233 100644 --- a/upup/pkg/fi/cloudup/gcetasks/projectiambinding.go +++ b/upup/pkg/fi/cloudup/gcetasks/projectiambinding.go @@ -134,21 +134,23 @@ func (_ *ProjectIAMBinding) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Projec return nil } -// terraformProjectIAMBinding is the model for a terraform google_project_iam_binding rule -type terraformProjectIAMBinding struct { - Project string `cty:"project"` - Role string `cty:"role"` - Members []*terraformWriter.Literal `cty:"members"` +// terraformProjectIAMMember is the model for a terraform google_project_iam_member rule. +type terraformProjectIAMMember struct { + Project string `cty:"project"` + Role string `cty:"role"` + Member *terraformWriter.Literal `cty:"member"` } func (_ *ProjectIAMBinding) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *ProjectIAMBinding) error { - tf := &terraformProjectIAMBinding{ + // Render an additive member to match the API target and avoid owning all + // principals granted the role on the project. + tf := &terraformProjectIAMMember{ Project: fi.ValueOf(e.Project), Role: fi.ValueOf(e.Role), - Members: []*terraformWriter.Literal{e.MemberServiceAccount.TerraformLink_Member()}, + Member: e.MemberServiceAccount.TerraformLink_Member(), } - return t.RenderResource("google_project_iam_binding", *e.Name, tf) + return t.RenderResource("google_project_iam_member", *e.Name, tf) } func patchCRMPolicy(policy *cloudresourcemanager.Policy, wantMember string, wantRole string) bool { diff --git a/upup/pkg/fi/cloudup/gcetasks/projectiambinding_test.go b/upup/pkg/fi/cloudup/gcetasks/projectiambinding_test.go index 26ed07b95e3b0..a7b07485aa8db 100644 --- a/upup/pkg/fi/cloudup/gcetasks/projectiambinding_test.go +++ b/upup/pkg/fi/cloudup/gcetasks/projectiambinding_test.go @@ -18,10 +18,14 @@ package gcetasks import ( "context" + "os" + "path/filepath" + "strings" "testing" gcemock "k8s.io/kops/cloudmock/gce" "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/upup/pkg/fi/cloudup/terraform" ) func TestProjectIAMBinding(t *testing.T) { @@ -69,3 +73,39 @@ func TestProjectIAMBinding(t *testing.T) { checkNoChanges(t, ctx, cloud, allTasks) } } + +func TestProjectIAMBindingRenderTerraform(t *testing.T) { + outDir := t.TempDir() + cloud := gcemock.InstallMockGCECloud("us-test1", "testproject") + target := terraform.NewTerraformTarget(cloud, "testproject", outDir, nil) + task := &ProjectIAMBinding{ + Name: fi.PtrTo("serviceaccount-nodes"), + Project: fi.PtrTo("testproject"), + MemberServiceAccount: &ServiceAccount{ + Name: fi.PtrTo("node"), + }, + Role: fi.PtrTo("roles/compute.viewer"), + } + + if err := task.RenderTerraform(target, nil, task, task); err != nil { + t.Fatalf("RenderTerraform() error: %v", err) + } + if err := target.Finish(nil); err != nil { + t.Fatalf("Finish() error: %v", err) + } + + b, err := os.ReadFile(filepath.Join(outDir, "kubernetes.tf")) + if err != nil { + t.Fatalf("reading rendered Terraform: %v", err) + } + rendered := string(b) + if !strings.Contains(rendered, `resource "google_project_iam_member" "serviceaccount-nodes"`) { + t.Errorf("rendered Terraform does not contain google_project_iam_member:\n%s", rendered) + } + if !strings.Contains(rendered, `member = format("serviceAccount:%s", google_service_account.node.email)`) { + t.Errorf("rendered Terraform does not contain the service account member expression:\n%s", rendered) + } + if strings.Contains(rendered, "google_project_iam_binding") || strings.Contains(rendered, "members =") { + t.Errorf("rendered Terraform contains authoritative IAM binding syntax:\n%s", rendered) + } +}