Harden GCE Terraform generation and add equivalence coverage

cmd/kops/integration_test.go

@@ -58,6 +58,8 @@ type integrationTest struct {
 	clusterName    string
 	srcDir         string
 	version        string
+	kopsVersion    string
+	keysetsFixture string
 	private        bool
 	zones          int
 	expectPolicies bool
@@ -75,6 +77,11 @@ type integrationTest struct {
 	nthRebalance bool
 	// enable GCE startup script
 	startupScript bool
+	// GCE instance group names override the legacy names derived from zones.
+	gceControlPlaneInstanceGroups []string
+	gceNodeInstanceGroups         []string
+	gceProject                    string
+	gceRegion                     string
 	// verify "kops get assets" functionality
 	testGetAssets bool
 }
@@ -106,11 +113,33 @@ func (i *integrationTest) withVersion(version string) *integrationTest {
 	return i
 }
 
+func (i *integrationTest) withKopsVersion(version string) *integrationTest {
+	i.kopsVersion = version
+	return i
+}
+
+func (i *integrationTest) withKeysetsFixture(filename string) *integrationTest {
+	i.keysetsFixture = filename
+	return i
+}
+
 func (i *integrationTest) withZones(zones int) *integrationTest {
 	i.zones = zones
 	return i
 }
 
+func (i *integrationTest) withGCEInstanceGroups(controlPlane, nodes []string) *integrationTest {
+	i.gceControlPlaneInstanceGroups = controlPlane
+	i.gceNodeInstanceGroups = nodes
+	return i
+}
+
+func (i *integrationTest) withGCEProject(project, region string) *integrationTest {
+	i.gceProject = project
+	i.gceRegion = region
+	return i
+}
+
 func (i *integrationTest) withoutSSHKey() *integrationTest {
 	i.sshKey = false
 	return i
@@ -441,6 +470,25 @@ func TestMinimalGCEDNSNone(t *testing.T) {
 		runTestTerraformGCE(t)
 }
 
+func TestGCEKopsModuleEquivalence(t *testing.T) {
+	newIntegrationTest("k8s1.equivalence.example.internal", "gce-kops-module-equivalence").
+		withoutSSHKey().
+		withKopsVersion("1.35.1").
+		withKeysetsFixture("golden-pki.json").
+		withGCEProject("example-project", "us-west1").
+		withGCEInstanceGroups(
+			[]string{"control-plane-us-west1-a", "control-plane-us-west1-b", "control-plane-us-west1-c"},
+			[]string{"nodes-us-west1-a", "nodes-us-west1-b", "nodes-us-west1-c"},
+		).
+		withAddons(
+			ciliumAddon,
+			gcpCCMAddon,
+			gcpPDCSIAddon,
+			metricsServerAddon,
+		).
+		runTestTerraformGCE(t)
+}
+
 // TestMinimalScaleway runs tests on a minimal Scaleway cluster with gossip DNS
 func TestMinimalScaleway(t *testing.T) {
 	t.Setenv("SCW_PROFILE", "REDACTED")
@@ -1377,6 +1425,11 @@ func (i *integrationTest) setupCluster(t *testing.T, ctx context.Context, inputY
 		t.Fatalf("error getting keystore: %v", err)
 	}
 
+	if i.keysetsFixture != "" {
+		i.storeKeysetsFixture(t, ctx, keyStore)
+		return factory
+	}
+
 	storeKeyset(t, ctx, keyStore, fi.CertificateIDCA, &testingKeyset{
 		primaryKey:           "-----BEGIN RSA PRIVATE KEY-----\nMIIBOgIBAAJBANFI3zr0Tk8krsW8vwjfMpzJOlWQ8616vG3YPa2qAgI7V4oKwfV0\nyIg1jt+H6f4P/wkPAPTPTfRp9Iy8oHEEFw0CAwEAAQJATmTyoZ3D+6dtBErocEVT\nKyHBhS3P6YrRLIBU0kmdiQHN8BuzvENqm5PASTq1m6yAAJs7qu9S0kO8u4G+SILv\n7QIhAPNCeJoFHmNUwQ1kxuta1RqICGcNoA4Yx5LiHXd9dPM7AiEA3D7gq8WB8csD\nghBNu/zLy3RdFCkfJqWkX5FhdX29alcCIHw4A1HTL1NV4kcuoQ1qEsw7jt7g7EyG\nhtMQuC9eVywlAiA1Z12s6Og4S+Se3fsrUQHNZHrJT6tJALMZpTO/fGy4YwIhANlJ\nR6hkVKtJp9zhipu6WpvpiAtoIlsNnPMPyuDRwV/u\n-----END RSA PRIVATE KEY-----",
 		primaryCertificate:   "-----BEGIN CERTIFICATE-----\nMIIBbjCCARigAwIBAgIMFpANqBD8NSD82AUSMA0GCSqGSIb3DQEBCwUAMBgxFjAU\nBgNVBAMTDWt1YmVybmV0ZXMtY2EwHhcNMjEwNzA3MDcwODAwWhcNMzEwNzA3MDcw\nODAwWjAYMRYwFAYDVQQDEw1rdWJlcm5ldGVzLWNhMFwwDQYJKoZIhvcNAQEBBQAD\nSwAwSAJBANFI3zr0Tk8krsW8vwjfMpzJOlWQ8616vG3YPa2qAgI7V4oKwfV0yIg1\njt+H6f4P/wkPAPTPTfRp9Iy8oHEEFw0CAwEAAaNCMEAwDgYDVR0PAQH/BAQDAgEG\nMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFNG3zVjTcLlJwDsJ4/K9DV7KohUA\nMA0GCSqGSIb3DQEBCwUAA0EAB8d03fY2w7WKpfO29qI295pu2C4ca9AiVGOpgSc8\ntmQsq6rcxt3T+rb589PVtz0mw/cKTxOk6gH2CCC+yHfy2w==\n-----END CERTIFICATE-----",
@@ -1449,9 +1502,78 @@ func (i *integrationTest) setupCluster(t *testing.T, ctx context.Context, inputY
 	return factory
 }
 
+type keysetsFixture struct {
+	CACertificatesPEM       map[string]string `json:"ca_certificates_pem"`
+	PrivateKeysPEM          map[string]string `json:"private_keys_pem"`
+	KeypairIDs              map[string]string `json:"keypair_ids"`
+	ServiceAccountPublicKey string            `json:"service_account_public_keys"`
+}
+
+func (i *integrationTest) storeKeysetsFixture(t *testing.T, ctx context.Context, keyStore fi.Keystore) {
+	b, err := os.ReadFile(path.Join(i.srcDir, i.keysetsFixture))
+	if err != nil {
+		t.Fatalf("reading keysets fixture: %v", err)
+	}
+	var fixture keysetsFixture
+	if err := json.Unmarshal(b, &fixture); err != nil {
+		t.Fatalf("decoding keysets fixture: %v", err)
+	}
+	keys := func(m map[string]string) []string {
+		out := make([]string, 0, len(m))
+		for key := range m {
+			out = append(out, key)
+		}
+		sort.Strings(out)
+		return out
+	}
+	names := keys(fixture.CACertificatesPEM)
+	if len(names) == 0 {
+		t.Fatalf("keysets fixture contains no keysets")
+	}
+	if !reflect.DeepEqual(names, keys(fixture.PrivateKeysPEM)) || !reflect.DeepEqual(names, keys(fixture.KeypairIDs)) {
+		t.Fatalf("keysets fixture map keys do not match")
+	}
+	if fixture.ServiceAccountPublicKey == "" {
+		t.Fatalf("keysets fixture service_account_public_keys is empty")
+	}
+	serviceAccountKeyPEM, ok := fixture.PrivateKeysPEM["service-account"]
+	if !ok {
+		t.Fatalf("keysets fixture does not contain service-account")
+	}
+	serviceAccountKey, err := pki.ParsePEMPrivateKey([]byte(serviceAccountKeyPEM))
+	if err != nil {
+		t.Fatalf("loading service-account private key: %v", err)
+	}
+	if serviceAccountKey == nil {
+		t.Fatalf("loading service-account private key: no key found")
+	}
+	publicKeyDER, err := x509.MarshalPKIXPublicKey(serviceAccountKey.Key.Public())
+	if err != nil {
+		t.Fatalf("marshalling service-account public key: %v", err)
+	}
+	var publicKeyPEM bytes.Buffer
+	if err := pem.Encode(&publicKeyPEM, &pem.Block{Type: "RSA PUBLIC KEY", Bytes: publicKeyDER}); err != nil {
+		t.Fatalf("encoding service-account public key: %v", err)
+	}
+	if publicKeyPEM.String() != fixture.ServiceAccountPublicKey {
+		t.Fatalf("keysets fixture service_account_public_keys does not match service-account private key")
+	}
+	for _, name := range names {
+		if fixture.CACertificatesPEM[name] == "" || fixture.PrivateKeysPEM[name] == "" || fixture.KeypairIDs[name] == "" {
+			t.Fatalf("keysets fixture keyset %q has an empty required value", name)
+		}
+		storeKeyset(t, ctx, keyStore, name, &testingKeyset{
+			primaryKey:         fixture.PrivateKeysPEM[name],
+			primaryCertificate: fixture.CACertificatesPEM[name],
+			primaryID:          fixture.KeypairIDs[name],
+		})
+	}
+}
+
 type testingKeyset struct {
 	primaryKey           string
 	primaryCertificate   string
+	primaryID            string
 	secondaryKey         string
 	secondaryCertificate string
 }
@@ -1472,6 +1594,11 @@ func storeKeyset(t *testing.T, ctx context.Context, keyStore fi.Keystore, name s
 		if err != nil {
 			t.Fatalf("error creating keyset: %v", err)
 		}
+		if testingKeyset.primaryID != "" {
+			delete(keyset.Items, keyset.Primary.Id)
+			keyset.Primary.Id = testingKeyset.primaryID
+			keyset.Items[keyset.Primary.Id] = keyset.Primary
+		}
 
 		if testingKeyset.secondaryKey != "" {
 			privateKey, err = pki.ParsePEMPrivateKey([]byte(testingKeyset.secondaryKey))
@@ -1640,25 +1767,25 @@ func (i *integrationTest) runTestTerraformGCE(t *testing.T) {
 	h := testutils.NewIntegrationTestHarness(t)
 	defer h.Close()
 
-	h.MockKopsVersion("1.34.0-beta.1")
-	h.SetupMockGCE()
-
-	expectedFilenames := i.expectTerraformFilenames
-
-	prefix := "google_compute_instance_template_nodes-" + gce.SafeClusterName(i.clusterName) + "_metadata_"
-	if !i.startupScript {
-		expectedFilenames = append(expectedFilenames, prefix+"user-data")
+	kopsVersion := i.kopsVersion
+	if kopsVersion == "" {
+		kopsVersion = "1.34.0-beta.1"
+	}
+	h.MockKopsVersion(kopsVersion)
+	if i.gceProject == "" {
+		h.SetupMockGCE()
 	} else {
-		expectedFilenames = append(expectedFilenames, prefix+"startup-script")
+		h.SetupMockGCEForProject(i.gceProject, i.gceRegion)
 	}
 
+	expectedFilenames := i.expectTerraformFilenames
+
 	expectedFilenames = append(expectedFilenames,
 		"aws_s3_object_cluster-completed.spec_content",
 		"aws_s3_object_etcd-cluster-spec-events_content",
 		"aws_s3_object_etcd-cluster-spec-main_content",
 		"aws_s3_object_kops-version.txt_content",
 		"aws_s3_object_manifests-static-kube-apiserver-healthcheck_content",
-		"aws_s3_object_nodeupconfig-nodes_content",
 		"aws_s3_object_"+i.clusterName+"-addons-bootstrap_content",
 		"aws_s3_object_"+i.clusterName+"-addons-coredns.addons.k8s.io-k8s-1.12_content",
 		"aws_s3_object_"+i.clusterName+"-addons-kops-controller.addons.k8s.io-k8s-1.16_content",
@@ -1667,14 +1794,34 @@ func (i *integrationTest) runTestTerraformGCE(t *testing.T) {
 		"aws_s3_object_"+i.clusterName+"-addons-storage-gce.addons.k8s.io-v1.7.0_content",
 	)
 
-	for j := 0; j < i.zones; j++ {
-		zone := "us-test1-" + string([]byte{byte('a') + byte(j)})
+	nodeInstanceGroups := i.gceNodeInstanceGroups
+	if nodeInstanceGroups == nil {
+		nodeInstanceGroups = []string{"nodes"}
+	}
+	for _, instanceGroup := range nodeInstanceGroups {
+		expectedFilenames = append(expectedFilenames, "aws_s3_object_nodeupconfig-"+instanceGroup+"_content")
 
-		expectedFilenames = append(expectedFilenames, "aws_s3_object_manifests-etcdmanager-events-master-"+zone+"_content")
-		expectedFilenames = append(expectedFilenames, "aws_s3_object_manifests-etcdmanager-main-master-"+zone+"_content")
-		expectedFilenames = append(expectedFilenames, "aws_s3_object_nodeupconfig-master-"+zone+"_content")
+		prefix := "google_compute_instance_template_" + instanceGroup + "-" + gce.SafeClusterName(i.clusterName) + "_metadata_"
+		if !i.startupScript {
+			expectedFilenames = append(expectedFilenames, prefix+"user-data")
+		} else {
+			expectedFilenames = append(expectedFilenames, prefix+"startup-script")
+		}
+	}
+
+	controlPlaneInstanceGroups := i.gceControlPlaneInstanceGroups
+	if controlPlaneInstanceGroups == nil {
+		for j := 0; j < i.zones; j++ {
+			zone := "us-test1-" + string([]byte{byte('a') + byte(j)})
+			controlPlaneInstanceGroups = append(controlPlaneInstanceGroups, "master-"+zone)
+		}
+	}
+	for _, instanceGroup := range controlPlaneInstanceGroups {
+		expectedFilenames = append(expectedFilenames, "aws_s3_object_manifests-etcdmanager-events-"+instanceGroup+"_content")
+		expectedFilenames = append(expectedFilenames, "aws_s3_object_manifests-etcdmanager-main-"+instanceGroup+"_content")
+		expectedFilenames = append(expectedFilenames, "aws_s3_object_nodeupconfig-"+instanceGroup+"_content")
 
-		prefix := "google_compute_instance_template_master-" + zone + "-" + gce.SafeClusterName(i.clusterName) + "_metadata_"
+		prefix := "google_compute_instance_template_" + instanceGroup + "-" + gce.SafeClusterName(i.clusterName) + "_metadata_"
 		if !i.startupScript {
 			expectedFilenames = append(expectedFilenames, prefix+"user-data")
 		} else {

docs/terraform.md

@@ -83,6 +83,27 @@ $ terraform apply
 
 Wait for the cluster to initialize. If all goes well, you should have a working Kubernetes cluster!
 
+#### Migrating GCE project IAM resources
+
+GCE Terraform output now manages the control-plane and node service-account
+roles with additive `google_project_iam_member` resources instead of
+authoritative `google_project_iam_binding` resources. Existing Terraform state
+does not automatically associate the new resource type with the old addresses.
+
+After regenerating the Terraform configuration, but before planning or applying
+it, remove any old addresses that are present from Terraform state:
+
+```
+$ terraform state rm 'google_project_iam_binding.serviceaccount-control-plane'
+$ terraform state rm 'google_project_iam_binding.serviceaccount-nodes'
+```
+
+Removing these addresses from Terraform state does not remove the IAM grants
+from GCP. Review the next plan and confirm it creates the corresponding
+`google_project_iam_member` resources without destroying project IAM bindings.
+This prevents the old authoritative resources from removing the grants after
+the additive resources have been created.
+
 #### Editing the cluster
 
 It's possible to use Terraform to make changes to your infrastructure as defined by kOps. In the example below we'd like to change some cluster configs:

pkg/nodemodel/wellknownassets/kopsassets.go

@@ -43,6 +43,15 @@ var protokubeAsset map[architectures.Architecture]*assets.MirroredAsset
 // channelsAsset caches the channels binary download url/hash
 var channelsAsset map[architectures.Architecture]*assets.MirroredAsset
 
+// ResetCachesForTesting clears cached kOps asset locations after tests change global configuration.
+// It must not be called concurrently with asset lookup.
+func ResetCachesForTesting() {
+	kopsBaseURL = nil
+	nodeUpAsset = nil
+	protokubeAsset = nil
+	channelsAsset = nil
+}
+
 // BaseURL returns the base url for the distribution of kops - in particular for nodeup & docker images
 func BaseURL() (*url.URL, error) {
 	// returning cached value

pkg/testutils/integrationtestharness.go

@@ -56,6 +56,7 @@ import (
 	"k8s.io/kops/cloudmock/openstack/mocknetworking"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/featureflag"
+	"k8s.io/kops/pkg/nodemodel/wellknownassets"
 	"k8s.io/kops/pkg/pki"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
@@ -122,6 +123,7 @@ func (h *IntegrationTestHarness) Close() {
 
 	if h.originalKopsVersion != "" {
 		kopsroot.Version = h.originalKopsVersion
+		wellknownassets.ResetCachesForTesting()
 	}
 
 	if h.originalDefaultChannelBase != "" {
@@ -284,9 +286,11 @@ func (h *IntegrationTestHarness) SetupMockAWS() *awsup.MockAWSCloud {
 
 // SetupMockGCE configures a mock GCE cloud provider
 func (h *IntegrationTestHarness) SetupMockGCE() *gcemock.MockGCECloud {
-	project := "testproject"
-	region := "us-test1"
+	return h.SetupMockGCEForProject("testproject", "us-test1")
+}
 
+// SetupMockGCEForProject configures a mock GCE cloud provider for the specified project and region.
+func (h *IntegrationTestHarness) SetupMockGCEForProject(project, region string) *gcemock.MockGCECloud {
 	cloud := gcemock.InstallMockGCECloud(region, project)
 
 	cloud.Compute().Networks().Insert(project, &compute.Network{
@@ -369,4 +373,5 @@ func (h *IntegrationTestHarness) MockKopsVersion(version string) {
 
 	h.originalKopsVersion = kopsroot.Version
 	kopsroot.Version = version
+	wellknownassets.ResetCachesForTesting()
 }

tests/integration/update_cluster/gce-kops-module-equivalence/README.md

@@ -0,0 +1,17 @@
+# GCE kops-module equivalence fixture
+
+This offline fixture exercises the GCE topology used by kops-module with
+generic cluster and project identities. Regenerate it with:
+
+```
+HACK_UPDATE_EXPECTED_IN_PLACE=1 go test ./cmd/kops \
+  -run '^TestGCEKopsModuleEquivalence$' -count=1
+```
+
+`golden-pki.json` contains public, deterministic test keys. They are committed
+only to make generated node configuration reproducible and must never be used
+for a real cluster.
+
+The `aws_s3_object_*` files represent objects in the integration harness's
+in-memory state store. They are expected for GCE Terraform fixtures and do not
+exercise AWS infrastructure.