Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions data/data/aws/bootstrap/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
{
Expand Down
6 changes: 0 additions & 6 deletions data/data/aws/bootstrap/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
20 changes: 5 additions & 15 deletions data/data/aws/iam/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
{
Expand Down
6 changes: 0 additions & 6 deletions data/data/aws/iam/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down
5 changes: 1 addition & 4 deletions data/data/aws/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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}"
Expand All @@ -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",
Expand Down
20 changes: 5 additions & 15 deletions data/data/aws/master/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
{
Expand Down
6 changes: 0 additions & 6 deletions data/data/aws/master/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
28 changes: 0 additions & 28 deletions data/data/aws/variables-aws.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 0 additions & 2 deletions docs/user/aws/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,7 +28,6 @@ machines:
- name: worker
platform:
aws:
iamRoleName: elastictranscoder-access
rootVolume:
iops: 4000
size: 500
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/installconfig/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions pkg/asset/installconfig/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestInstallConfigLoad(t *testing.T) {
{
name: "valid InstallConfig",
data: `
apiVersion: v1beta1
apiVersion: v1beta2
metadata:
name: test-cluster
baseDomain: test-domain
Expand All @@ -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",
Expand Down
7 changes: 0 additions & 7 deletions pkg/tfvars/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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"`
}
11 changes: 1 addition & 10 deletions pkg/tfvars/tfvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
7 changes: 0 additions & 7 deletions pkg/types/aws/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/types/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions pkg/types/validation/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/validation/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down