pkg/types/aws/machinepool: Drop IAM-role overrides

wking · wking · commit 3b393da8b9db · 2019-02-01T00:50:37.000-08:00
We're planning on dropping instance profiles in favor of the new credentials operator [1], because we want AWS access to have operator/pod/namespace granularity and not instance granularity. Many pods could be running on a given instance, and not all of them should have a given permission. While we're blocked from dropping these at the moment due to kubelet cloud-config+secrets [2], we can drop the user-facing knobs for this feature now. Then pivoting the internal approach once we get the kubelet sorted will be a non-breaking change. [1]: https://github.com/openshift/cloud-credential-operator [2]: #697 (comment)
diff --git a/data/data/aws/bootstrap/main.tf b/data/data/aws/bootstrap/main.tf
@@ -32,21 +32,12 @@ data "ignition_config" "redirect" {
 resource "aws_iam_instance_profile" "bootstrap" {
   name = "${var.cluster_name}-bootstrap-profile"
 
-  role = "${var.iam_role == "" ?
-    join("|", aws_iam_role.bootstrap.*.name) :
-    join("|", data.aws_iam_role.bootstrap.*.name)
-  }"
-}
-
-data "aws_iam_role" "bootstrap" {
-  count = "${var.iam_role == "" ? 0 : 1}"
-  name  = "${var.iam_role}"
+  role = "${aws_iam_role.bootstrap.name}"
 }
 
 resource "aws_iam_role" "bootstrap" {
-  count = "${var.iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}-bootstrap-role"
-  path  = "/"
+  name = "${var.cluster_name}-bootstrap-role"
+  path = "/"
 
   assume_role_policy = <<EOF
 {
@@ -68,9 +59,8 @@ EOF
 }
 
 resource "aws_iam_role_policy" "bootstrap" {
-  count = "${var.iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}-bootstrap-policy"
-  role  = "${aws_iam_role.bootstrap.id}"
+  name = "${var.cluster_name}-bootstrap-policy"
+  role = "${aws_iam_role.bootstrap.id}"
 
   policy = <<EOF
 {
diff --git a/data/data/aws/bootstrap/variables.tf b/data/data/aws/bootstrap/variables.tf
@@ -8,12 +8,6 @@ variable "cluster_name" {
   description = "The name of the cluster."
 }
 
-variable "iam_role" {
-  type        = "string"
-  default     = ""
-  description = "The name of the IAM role to assign to the bootstrap node."
-}
-
 variable "ignition" {
   type        = "string"
   description = "The content of the bootstrap ignition file."
diff --git a/data/data/aws/iam/main.tf b/data/data/aws/iam/main.tf
@@ -5,21 +5,12 @@ locals {
 resource "aws_iam_instance_profile" "worker" {
   name = "${var.cluster_name}-worker-profile"
 
-  role = "${var.worker_iam_role == "" ?
-    join("|", aws_iam_role.worker_role.*.name) :
-    join("|", data.aws_iam_role.worker_role.*.name)
-  }"
-}
-
-data "aws_iam_role" "worker_role" {
-  count = "${var.worker_iam_role == "" ? 0 : 1}"
-  name  = "${var.worker_iam_role}"
+  role = "${aws_iam_role.worker_role.name}"
 }
 
 resource "aws_iam_role" "worker_role" {
-  count = "${var.worker_iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}-worker-role"
-  path  = "/"
+  name = "${var.cluster_name}-worker-role"
+  path = "/"
 
   assume_role_policy = <<EOF
 {
@@ -41,9 +32,8 @@ EOF
 }
 
 resource "aws_iam_role_policy" "worker_policy" {
-  count = "${var.worker_iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}_worker_policy"
-  role  = "${aws_iam_role.worker_role.id}"
+  name = "${var.cluster_name}_worker_policy"
+  role = "${aws_iam_role.worker_role.id}"
 
   policy = <<EOF
 {
diff --git a/data/data/aws/iam/variables.tf b/data/data/aws/iam/variables.tf
@@ -2,12 +2,6 @@ variable "cluster_name" {
   type = "string"
 }
 
-variable "worker_iam_role" {
-  type        = "string"
-  default     = ""
-  description = "IAM role to use for the instance profiles of worker nodes."
-}
-
 variable "tags" {
   type        = "map"
   default     = {}
diff --git a/data/data/aws/main.tf b/data/data/aws/main.tf
@@ -15,7 +15,6 @@ module "bootstrap" {
 
   ami                      = "${var.aws_ec2_ami_override}"
   cluster_name             = "${var.cluster_name}"
-  iam_role                 = "${var.aws_master_iam_role_name}"
   ignition                 = "${var.ignition_bootstrap}"
   subnet_id                = "${module.vpc.master_subnet_ids[0]}"
   target_group_arns        = "${module.vpc.aws_lb_target_group_arns}"
@@ -40,7 +39,6 @@ module "masters" {
     ), local.tags)}"
 
   instance_count           = "${var.master_count}"
-  master_iam_role          = "${var.aws_master_iam_role_name}"
   master_sg_ids            = "${list(module.vpc.master_sg_id)}"
   root_volume_iops         = "${var.aws_master_root_volume_iops}"
   root_volume_size         = "${var.aws_master_root_volume_size}"
@@ -55,8 +53,7 @@ module "masters" {
 module "iam" {
   source = "./iam"
 
-  cluster_name    = "${var.cluster_name}"
-  worker_iam_role = "${var.aws_worker_iam_role_name}"
+  cluster_name = "${var.cluster_name}"
 
   tags = "${merge(map(
       "kubernetes.io/cluster/${var.cluster_name}", "owned",
diff --git a/data/data/aws/master/main.tf b/data/data/aws/master/main.tf
@@ -5,21 +5,12 @@ locals {
 resource "aws_iam_instance_profile" "master" {
   name = "${var.cluster_name}-master-profile"
 
-  role = "${var.master_iam_role == "" ?
-    join("|", aws_iam_role.master_role.*.name) :
-    join("|", data.aws_iam_role.master_role.*.name)
-  }"
-}
-
-data "aws_iam_role" "master_role" {
-  count = "${var.master_iam_role == "" ? 0 : 1}"
-  name  = "${var.master_iam_role}"
+  role = "${aws_iam_role.master_role.name}"
 }
 
 resource "aws_iam_role" "master_role" {
-  count = "${var.master_iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}-master-role"
-  path  = "/"
+  name = "${var.cluster_name}-master-role"
+  path = "/"
 
   assume_role_policy = <<EOF
 {
@@ -41,9 +32,8 @@ EOF
 }
 
 resource "aws_iam_role_policy" "master_policy" {
-  count = "${var.master_iam_role == "" ? 1 : 0}"
-  name  = "${var.cluster_name}_master_policy"
-  role  = "${aws_iam_role.master_role.id}"
+  name = "${var.cluster_name}_master_policy"
+  role = "${aws_iam_role.master_role.id}"
 
   policy = <<EOF
 {
diff --git a/data/data/aws/master/variables.tf b/data/data/aws/master/variables.tf
@@ -29,12 +29,6 @@ variable "kubeconfig_content" {
   default = ""
 }
 
-variable "master_iam_role" {
-  type        = "string"
-  default     = ""
-  description = "IAM role to use for the instance profiles of master nodes."
-}
-
 variable "master_sg_ids" {
   type        = "list"
   description = "The security group IDs to be applied to the master nodes."
diff --git a/data/data/aws/variables-aws.tf b/data/data/aws/variables-aws.tf
@@ -59,31 +59,3 @@ variable "aws_region" {
   type        = "string"
   description = "The target AWS region for the cluster."
 }
-
-variable "aws_master_iam_role_name" {
-  type    = "string"
-  default = ""
-
-  description = <<EOF
-(optional) Name of IAM role to use for the instance profiles of master nodes.
-The name is also the last part of a role's ARN.
-
-Example:
- * Role ARN  = arn:aws:iam::123456789012:role/openshift-installer
- * Role Name = openshift-installer
-EOF
-}
-
-variable "aws_worker_iam_role_name" {
-  type    = "string"
-  default = ""
-
-  description = <<EOF
-(optional) Name of IAM role to use for the instance profiles of worker nodes.
-The name is also the last part of a role's ARN.
-
-Example:
- * Role ARN  = arn:aws:iam::123456789012:role/openshift-installer
- * Role Name = openshift-installer
-EOF
-}
diff --git a/docs/user/aws/customization.md b/docs/user/aws/customization.md
@@ -2,7 +2,6 @@
 
 The following options are available when using AWS:
 
-- `machines.platform.aws.iamRoleName` - the IAM role that will be assigned to the machines of this pool
 - `machines.platform.aws.rootVolume.iops` - the reserved IOPS of the root volume
 - `machines.platform.aws.rootVolume.size` - the size (in GiB) of the root volume
 - `machines.platform.aws.rootVolume.type` - the storage type of the root volume
@@ -29,7 +28,6 @@ machines:
 - name: worker
   platform:
     aws:
-      iamRoleName: elastictranscoder-access
       rootVolume:
         iops: 4000
         size: 500
diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go
@@ -55,7 +55,7 @@ func (a *InstallConfig) Generate(parents asset.Parents) error {
 
 	a.Config = &types.InstallConfig{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1beta1",
+			APIVersion: "v1beta2",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name: clusterName.ClusterName,
diff --git a/pkg/asset/installconfig/installconfig_test.go b/pkg/asset/installconfig/installconfig_test.go
@@ -20,7 +20,7 @@ import (
 func validInstallConfig() *types.InstallConfig {
 	return &types.InstallConfig{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1beta1",
+			APIVersion: types.InstallConfigVersion,
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-cluster",
@@ -57,7 +57,7 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
 	}
 	expected := &types.InstallConfig{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1beta1",
+			APIVersion: types.InstallConfigVersion,
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-cluster",
@@ -104,7 +104,7 @@ func TestInstallConfigLoad(t *testing.T) {
 		{
 			name: "valid InstallConfig",
 			data: `
-apiVersion: v1beta1
+apiVersion: v1beta2
 metadata:
   name: test-cluster
 baseDomain: test-domain
@@ -116,7 +116,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
 			expectedFound: true,
 			expectedConfig: &types.InstallConfig{
 				TypeMeta: metav1.TypeMeta{
-					APIVersion: "v1beta1",
+					APIVersion: types.InstallConfigVersion,
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test-cluster",
diff --git a/pkg/tfvars/aws/aws.go b/pkg/tfvars/aws/aws.go
@@ -6,13 +6,11 @@ type AWS struct {
 	ExtraTags      map[string]string `json:"aws_extra_tags,omitempty"`
 	Master         `json:",inline"`
 	Region         string `json:"aws_region,omitempty"`
-	Worker         `json:",inline"`
 }
 
 // Master converts master related config.
 type Master struct {
 	EC2Type          string `json:"aws_master_ec2_type,omitempty"`
-	IAMRoleName      string `json:"aws_master_iam_role_name,omitempty"`
 	MasterRootVolume `json:",inline"`
 }
 
@@ -22,8 +20,3 @@ type MasterRootVolume struct {
 	Size int    `json:"aws_master_root_volume_size,omitempty"`
 	Type string `json:"aws_master_root_volume_type,omitempty"`
 }
-
-// Worker converts worker related config.
-type Worker struct {
-	IAMRoleName string `json:"aws_worker_iam_role_name,omitempty"`
-}
diff --git a/pkg/tfvars/tfvars.go b/pkg/tfvars/tfvars.go
@@ -52,23 +52,14 @@ func TFVars(clusterID string, cfg *types.InstallConfig, osImage, bootstrapIgn, m
 			config.Masters += replicas
 			if m.Platform.AWS != nil {
 				config.AWS.Master = aws.Master{
-					EC2Type:     m.Platform.AWS.InstanceType,
-					IAMRoleName: m.Platform.AWS.IAMRoleName,
+					EC2Type: m.Platform.AWS.InstanceType,
 					MasterRootVolume: aws.MasterRootVolume{
 						IOPS: m.Platform.AWS.EC2RootVolume.IOPS,
 						Size: m.Platform.AWS.EC2RootVolume.Size,
 						Type: m.Platform.AWS.EC2RootVolume.Type,
 					},
 				}
 			}
-		case "worker":
-			if m.Platform.AWS != nil {
-				config.AWS.Worker = aws.Worker{
-					IAMRoleName: m.Platform.AWS.IAMRoleName,
-				}
-			}
-		default:
-			return nil, errors.Errorf("unrecognized machine pool %q", m.Name)
 		}
 	}
 
diff --git a/pkg/types/aws/machinepool.go b/pkg/types/aws/machinepool.go
@@ -10,10 +10,6 @@ type MachinePool struct {
 	// eg. m4-large
 	InstanceType string `json:"type"`
 
-	// IAMRoleName defines the IAM role associated
-	// with the ec2 instance.
-	IAMRoleName string `json:"iamRoleName"`
-
 	// EC2RootVolume defines the storage for ec2 instance.
 	EC2RootVolume `json:"rootVolume"`
 }
@@ -31,9 +27,6 @@ func (a *MachinePool) Set(required *MachinePool) {
 	if required.InstanceType != "" {
 		a.InstanceType = required.InstanceType
 	}
-	if required.IAMRoleName != "" {
-		a.IAMRoleName = required.IAMRoleName
-	}
 
 	if required.EC2RootVolume.IOPS != 0 {
 		a.EC2RootVolume.IOPS = required.EC2RootVolume.IOPS
diff --git a/pkg/types/installconfig.go b/pkg/types/installconfig.go
@@ -9,6 +9,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	// InstallConfigVersion is the version supported by this package.
+	InstallConfigVersion = "v1beta2"
+)
+
 var (
 	// PlatformNames is a slice with all the visibly-supported
 	// platform names in alphabetical order. This is the list of
diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go
@@ -18,18 +18,14 @@ import (
 	"github.com/openshift/installer/pkg/validate"
 )
 
-const (
-	installConfigVersion = "v1beta1"
-)
-
 // ValidateInstallConfig checks that the specified install config is valid.
 func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher openstackvalidation.ValidValuesFetcher) field.ErrorList {
 	allErrs := field.ErrorList{}
 	if c.TypeMeta.APIVersion == "" {
 		return field.ErrorList{field.Required(field.NewPath("apiVersion"), "install-config version required")}
 	}
-	if c.TypeMeta.APIVersion != installConfigVersion {
-		return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), c.TypeMeta.APIVersion, fmt.Sprintf("install-config version must be %q", installConfigVersion))}
+	if c.TypeMeta.APIVersion != types.InstallConfigVersion && c.TypeMeta.APIVersion != "v1beta1" { // FIXME: v1beta1 is a temporary hack to get CI across the transition
+		return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), c.TypeMeta.APIVersion, fmt.Sprintf("install-config version must be %q", types.InstallConfigVersion))}
 	}
 	if c.ObjectMeta.Name == "" {
 		allErrs = append(allErrs, field.Required(field.NewPath("metadata", "name"), "cluster name required"))
diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go
@@ -18,7 +18,7 @@ import (
 func validInstallConfig() *types.InstallConfig {
 	return &types.InstallConfig{
 		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1beta1",
+			APIVersion: types.InstallConfigVersion,
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-cluster",
