Skip to content

Commit 3b393da

Browse files
wkingwking
committed
pkg/types/aws/machinepool: Drop IAM-role overrides
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)
1 parent 810b13a commit 3b393da

File tree

17 files changed

+30
-133
lines changed

17 files changed

+30
-133
lines changed

‎data/data/aws/bootstrap/main.tf

+5-15
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,12 @@ data "ignition_config" "redirect" {
3232
resource "aws_iam_instance_profile" "bootstrap" {
3333
name = "${var.cluster_name}-bootstrap-profile"
3434

35-
role = "${var.iam_role == "" ?
36-
join("|", aws_iam_role.bootstrap.*.name) :
37-
join("|", data.aws_iam_role.bootstrap.*.name)
38-
}"
39-
}
40-
41-
data "aws_iam_role" "bootstrap" {
42-
count = "${var.iam_role == "" ? 0 : 1}"
43-
name = "${var.iam_role}"
35+
role = "${aws_iam_role.bootstrap.name}"
4436
}
4537

4638
resource "aws_iam_role" "bootstrap" {
47-
count = "${var.iam_role == "" ? 1 : 0}"
48-
name = "${var.cluster_name}-bootstrap-role"
49-
path = "/"
39+
name = "${var.cluster_name}-bootstrap-role"
40+
path = "/"
5041

5142
assume_role_policy = <<EOF
5243
{
@@ -68,9 +59,8 @@ EOF
6859
}
6960

7061
resource "aws_iam_role_policy" "bootstrap" {
71-
count = "${var.iam_role == "" ? 1 : 0}"
72-
name = "${var.cluster_name}-bootstrap-policy"
73-
role = "${aws_iam_role.bootstrap.id}"
62+
name = "${var.cluster_name}-bootstrap-policy"
63+
role = "${aws_iam_role.bootstrap.id}"
7464

7565
policy = <<EOF
7666
{

‎data/data/aws/bootstrap/variables.tf

-6
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,6 @@ variable "cluster_name" {
88
description = "The name of the cluster."
99
}
1010

11-
variable "iam_role" {
12-
type = "string"
13-
default = ""
14-
description = "The name of the IAM role to assign to the bootstrap node."
15-
}
16-
1711
variable "ignition" {
1812
type = "string"
1913
description = "The content of the bootstrap ignition file."

‎data/data/aws/iam/main.tf

+5-15
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,12 @@ locals {
55
resource "aws_iam_instance_profile" "worker" {
66
name = "${var.cluster_name}-worker-profile"
77

8-
role = "${var.worker_iam_role == "" ?
9-
join("|", aws_iam_role.worker_role.*.name) :
10-
join("|", data.aws_iam_role.worker_role.*.name)
11-
}"
12-
}
13-
14-
data "aws_iam_role" "worker_role" {
15-
count = "${var.worker_iam_role == "" ? 0 : 1}"
16-
name = "${var.worker_iam_role}"
8+
role = "${aws_iam_role.worker_role.name}"
179
}
1810

1911
resource "aws_iam_role" "worker_role" {
20-
count = "${var.worker_iam_role == "" ? 1 : 0}"
21-
name = "${var.cluster_name}-worker-role"
22-
path = "/"
12+
name = "${var.cluster_name}-worker-role"
13+
path = "/"
2314

2415
assume_role_policy = <<EOF
2516
{
@@ -41,9 +32,8 @@ EOF
4132
}
4233

4334
resource "aws_iam_role_policy" "worker_policy" {
44-
count = "${var.worker_iam_role == "" ? 1 : 0}"
45-
name = "${var.cluster_name}_worker_policy"
46-
role = "${aws_iam_role.worker_role.id}"
35+
name = "${var.cluster_name}_worker_policy"
36+
role = "${aws_iam_role.worker_role.id}"
4737

4838
policy = <<EOF
4939
{

‎data/data/aws/iam/variables.tf

-6
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@ variable "cluster_name" {
22
type = "string"
33
}
44

5-
variable "worker_iam_role" {
6-
type = "string"
7-
default = ""
8-
description = "IAM role to use for the instance profiles of worker nodes."
9-
}
10-
115
variable "tags" {
126
type = "map"
137
default = {}

‎data/data/aws/main.tf

+1-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ module "bootstrap" {
1515

1616
ami = "${var.aws_ec2_ami_override}"
1717
cluster_name = "${var.cluster_name}"
18-
iam_role = "${var.aws_master_iam_role_name}"
1918
ignition = "${var.ignition_bootstrap}"
2019
subnet_id = "${module.vpc.master_subnet_ids[0]}"
2120
target_group_arns = "${module.vpc.aws_lb_target_group_arns}"
@@ -40,7 +39,6 @@ module "masters" {
4039
), local.tags)}"
4140

4241
instance_count = "${var.master_count}"
43-
master_iam_role = "${var.aws_master_iam_role_name}"
4442
master_sg_ids = "${list(module.vpc.master_sg_id)}"
4543
root_volume_iops = "${var.aws_master_root_volume_iops}"
4644
root_volume_size = "${var.aws_master_root_volume_size}"
@@ -55,8 +53,7 @@ module "masters" {
5553
module "iam" {
5654
source = "./iam"
5755

58-
cluster_name = "${var.cluster_name}"
59-
worker_iam_role = "${var.aws_worker_iam_role_name}"
56+
cluster_name = "${var.cluster_name}"
6057

6158
tags = "${merge(map(
6259
"kubernetes.io/cluster/${var.cluster_name}", "owned",

‎data/data/aws/master/main.tf

+5-15
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,12 @@ locals {
55
resource "aws_iam_instance_profile" "master" {
66
name = "${var.cluster_name}-master-profile"
77

8-
role = "${var.master_iam_role == "" ?
9-
join("|", aws_iam_role.master_role.*.name) :
10-
join("|", data.aws_iam_role.master_role.*.name)
11-
}"
12-
}
13-
14-
data "aws_iam_role" "master_role" {
15-
count = "${var.master_iam_role == "" ? 0 : 1}"
16-
name = "${var.master_iam_role}"
8+
role = "${aws_iam_role.master_role.name}"
179
}
1810

1911
resource "aws_iam_role" "master_role" {
20-
count = "${var.master_iam_role == "" ? 1 : 0}"
21-
name = "${var.cluster_name}-master-role"
22-
path = "/"
12+
name = "${var.cluster_name}-master-role"
13+
path = "/"
2314

2415
assume_role_policy = <<EOF
2516
{
@@ -41,9 +32,8 @@ EOF
4132
}
4233

4334
resource "aws_iam_role_policy" "master_policy" {
44-
count = "${var.master_iam_role == "" ? 1 : 0}"
45-
name = "${var.cluster_name}_master_policy"
46-
role = "${aws_iam_role.master_role.id}"
35+
name = "${var.cluster_name}_master_policy"
36+
role = "${aws_iam_role.master_role.id}"
4737

4838
policy = <<EOF
4939
{

‎data/data/aws/master/variables.tf

-6
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ variable "kubeconfig_content" {
2929
default = ""
3030
}
3131

32-
variable "master_iam_role" {
33-
type = "string"
34-
default = ""
35-
description = "IAM role to use for the instance profiles of master nodes."
36-
}
37-
3832
variable "master_sg_ids" {
3933
type = "list"
4034
description = "The security group IDs to be applied to the master nodes."

‎data/data/aws/variables-aws.tf

-28
Original file line numberDiff line numberDiff line change
@@ -59,31 +59,3 @@ variable "aws_region" {
5959
type = "string"
6060
description = "The target AWS region for the cluster."
6161
}
62-
63-
variable "aws_master_iam_role_name" {
64-
type = "string"
65-
default = ""
66-
67-
description = <<EOF
68-
(optional) Name of IAM role to use for the instance profiles of master nodes.
69-
The name is also the last part of a role's ARN.
70-
71-
Example:
72-
* Role ARN = arn:aws:iam::123456789012:role/openshift-installer
73-
* Role Name = openshift-installer
74-
EOF
75-
}
76-
77-
variable "aws_worker_iam_role_name" {
78-
type = "string"
79-
default = ""
80-
81-
description = <<EOF
82-
(optional) Name of IAM role to use for the instance profiles of worker nodes.
83-
The name is also the last part of a role's ARN.
84-
85-
Example:
86-
* Role ARN = arn:aws:iam::123456789012:role/openshift-installer
87-
* Role Name = openshift-installer
88-
EOF
89-
}

‎docs/user/aws/customization.md

-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
The following options are available when using AWS:
44

5-
- `machines.platform.aws.iamRoleName` - the IAM role that will be assigned to the machines of this pool
65
- `machines.platform.aws.rootVolume.iops` - the reserved IOPS of the root volume
76
- `machines.platform.aws.rootVolume.size` - the size (in GiB) of the root volume
87
- `machines.platform.aws.rootVolume.type` - the storage type of the root volume
@@ -29,7 +28,6 @@ machines:
2928
- name: worker
3029
platform:
3130
aws:
32-
iamRoleName: elastictranscoder-access
3331
rootVolume:
3432
iops: 4000
3533
size: 500

‎pkg/asset/installconfig/installconfig.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (a *InstallConfig) Generate(parents asset.Parents) error {
5555

5656
a.Config = &types.InstallConfig{
5757
TypeMeta: metav1.TypeMeta{
58-
APIVersion: "v1beta1",
58+
APIVersion: "v1beta2",
5959
},
6060
ObjectMeta: metav1.ObjectMeta{
6161
Name: clusterName.ClusterName,

‎pkg/asset/installconfig/installconfig_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
func validInstallConfig() *types.InstallConfig {
2121
return &types.InstallConfig{
2222
TypeMeta: metav1.TypeMeta{
23-
APIVersion: "v1beta1",
23+
APIVersion: types.InstallConfigVersion,
2424
},
2525
ObjectMeta: metav1.ObjectMeta{
2626
Name: "test-cluster",
@@ -57,7 +57,7 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
5757
}
5858
expected := &types.InstallConfig{
5959
TypeMeta: metav1.TypeMeta{
60-
APIVersion: "v1beta1",
60+
APIVersion: types.InstallConfigVersion,
6161
},
6262
ObjectMeta: metav1.ObjectMeta{
6363
Name: "test-cluster",
@@ -104,7 +104,7 @@ func TestInstallConfigLoad(t *testing.T) {
104104
{
105105
name: "valid InstallConfig",
106106
data: `
107-
apiVersion: v1beta1
107+
apiVersion: v1beta2
108108
metadata:
109109
name: test-cluster
110110
baseDomain: test-domain
@@ -116,7 +116,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
116116
expectedFound: true,
117117
expectedConfig: &types.InstallConfig{
118118
TypeMeta: metav1.TypeMeta{
119-
APIVersion: "v1beta1",
119+
APIVersion: types.InstallConfigVersion,
120120
},
121121
ObjectMeta: metav1.ObjectMeta{
122122
Name: "test-cluster",

‎pkg/tfvars/aws/aws.go

-7
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,11 @@ type AWS struct {
66
ExtraTags map[string]string `json:"aws_extra_tags,omitempty"`
77
Master `json:",inline"`
88
Region string `json:"aws_region,omitempty"`
9-
Worker `json:",inline"`
109
}
1110

1211
// Master converts master related config.
1312
type Master struct {
1413
EC2Type string `json:"aws_master_ec2_type,omitempty"`
15-
IAMRoleName string `json:"aws_master_iam_role_name,omitempty"`
1614
MasterRootVolume `json:",inline"`
1715
}
1816

@@ -22,8 +20,3 @@ type MasterRootVolume struct {
2220
Size int `json:"aws_master_root_volume_size,omitempty"`
2321
Type string `json:"aws_master_root_volume_type,omitempty"`
2422
}
25-
26-
// Worker converts worker related config.
27-
type Worker struct {
28-
IAMRoleName string `json:"aws_worker_iam_role_name,omitempty"`
29-
}

‎pkg/tfvars/tfvars.go

+1-10
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,14 @@ func TFVars(clusterID string, cfg *types.InstallConfig, osImage, bootstrapIgn, m
5252
config.Masters += replicas
5353
if m.Platform.AWS != nil {
5454
config.AWS.Master = aws.Master{
55-
EC2Type: m.Platform.AWS.InstanceType,
56-
IAMRoleName: m.Platform.AWS.IAMRoleName,
55+
EC2Type: m.Platform.AWS.InstanceType,
5756
MasterRootVolume: aws.MasterRootVolume{
5857
IOPS: m.Platform.AWS.EC2RootVolume.IOPS,
5958
Size: m.Platform.AWS.EC2RootVolume.Size,
6059
Type: m.Platform.AWS.EC2RootVolume.Type,
6160
},
6261
}
6362
}
64-
case "worker":
65-
if m.Platform.AWS != nil {
66-
config.AWS.Worker = aws.Worker{
67-
IAMRoleName: m.Platform.AWS.IAMRoleName,
68-
}
69-
}
70-
default:
71-
return nil, errors.Errorf("unrecognized machine pool %q", m.Name)
7263
}
7364
}
7465

‎pkg/types/aws/machinepool.go

-7
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@ type MachinePool struct {
1010
// eg. m4-large
1111
InstanceType string `json:"type"`
1212

13-
// IAMRoleName defines the IAM role associated
14-
// with the ec2 instance.
15-
IAMRoleName string `json:"iamRoleName"`
16-
1713
// EC2RootVolume defines the storage for ec2 instance.
1814
EC2RootVolume `json:"rootVolume"`
1915
}
@@ -31,9 +27,6 @@ func (a *MachinePool) Set(required *MachinePool) {
3127
if required.InstanceType != "" {
3228
a.InstanceType = required.InstanceType
3329
}
34-
if required.IAMRoleName != "" {
35-
a.IAMRoleName = required.IAMRoleName
36-
}
3730

3831
if required.EC2RootVolume.IOPS != 0 {
3932
a.EC2RootVolume.IOPS = required.EC2RootVolume.IOPS

‎pkg/types/installconfig.go

+5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import (
99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010
)
1111

12+
const (
13+
// InstallConfigVersion is the version supported by this package.
14+
InstallConfigVersion = "v1beta2"
15+
)
16+
1217
var (
1318
// PlatformNames is a slice with all the visibly-supported
1419
// platform names in alphabetical order. This is the list of

‎pkg/types/validation/installconfig.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,14 @@ import (
1818
"github.com/openshift/installer/pkg/validate"
1919
)
2020

21-
const (
22-
installConfigVersion = "v1beta1"
23-
)
24-
2521
// ValidateInstallConfig checks that the specified install config is valid.
2622
func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher openstackvalidation.ValidValuesFetcher) field.ErrorList {
2723
allErrs := field.ErrorList{}
2824
if c.TypeMeta.APIVersion == "" {
2925
return field.ErrorList{field.Required(field.NewPath("apiVersion"), "install-config version required")}
3026
}
31-
if c.TypeMeta.APIVersion != installConfigVersion {
32-
return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), c.TypeMeta.APIVersion, fmt.Sprintf("install-config version must be %q", installConfigVersion))}
27+
if c.TypeMeta.APIVersion != types.InstallConfigVersion && c.TypeMeta.APIVersion != "v1beta1" { // FIXME: v1beta1 is a temporary hack to get CI across the transition
28+
return field.ErrorList{field.Invalid(field.NewPath("apiVersion"), c.TypeMeta.APIVersion, fmt.Sprintf("install-config version must be %q", types.InstallConfigVersion))}
3329
}
3430
if c.ObjectMeta.Name == "" {
3531
allErrs = append(allErrs, field.Required(field.NewPath("metadata", "name"), "cluster name required"))

‎pkg/types/validation/installconfig_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
func validInstallConfig() *types.InstallConfig {
1919
return &types.InstallConfig{
2020
TypeMeta: metav1.TypeMeta{
21-
APIVersion: "v1beta1",
21+
APIVersion: types.InstallConfigVersion,
2222
},
2323
ObjectMeta: metav1.ObjectMeta{
2424
Name: "test-cluster",

0 commit comments

Comments
 (0)