Skip to content
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

Add feature: Custom IAM Roles #2139

Closed
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
19 changes: 19 additions & 0 deletions cmd/kops/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type CreateClusterOptions struct {
MasterSize string
MasterCount int32
NodeCount int32
MasterIAMRole string
NodeIAMRole string
Project string
KubernetesVersion string
OutDir string
Expand Down Expand Up @@ -147,6 +149,10 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {

cmd.Flags().StringVar(&options.MasterSize, "master-size", options.MasterSize, "Set instance size for masters")

cmd.Flags().StringVar(&options.NodeIAMRole, "node-iam-role", options.NodeIAMRole, "Set IAM role for nodes")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that we leave this at a yaml / API / clusterspec level. Not sure if we want more cli flags.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it myself with the cli flags, but fine if it ends up in the yaml

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @sp-borja-juncosa is on vacations, but I guess that it could be easy to remove the cli flags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sp-andres-diaz that would be awesome ... please and thank-you


cmd.Flags().StringVar(&options.MasterIAMRole, "master-iam-role", options.MasterIAMRole, "Set IAM role for masters")

cmd.Flags().StringVar(&options.VPCID, "vpc", options.VPCID, "Set to use a shared VPC")
cmd.Flags().StringVar(&options.NetworkCIDR, "network-cidr", options.NetworkCIDR, "Set to override the default network CIDR")

Expand Down Expand Up @@ -466,6 +472,19 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}
}

var customIamRoles map[string]string
customIamRoles = make(map[string]string)

if c.MasterIAMRole != "" {
customIamRoles["Master"] = c.MasterIAMRole
}

if c.NodeIAMRole != "" {
customIamRoles["Node"] = c.NodeIAMRole
}

cluster.Spec.RoleCustomIamRoles = customIamRoles

if c.DNSZone != "" {
cluster.Spec.DNSZone = c.DNSZone
}
Expand Down
17 changes: 12 additions & 5 deletions cmd/kops/create_cluster_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ package main

import (
"bytes"
"github.com/golang/glog"
"io/ioutil"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kops/cmd/kops/util"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/diff"
"path"
"strings"
"testing"
"time"

"github.com/golang/glog"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kops/cmd/kops/util"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/diff"
)

var MagicTimestamp = metav1.Time{Time: time.Date(2017, 1, 1, 0, 0, 0, 0, time.UTC)}
Expand All @@ -38,6 +39,12 @@ func TestCreateClusterMinimal(t *testing.T) {
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/minimal", "v1alpha2")
}

// TestCreateClusterCustomIamRole runs kops create cluster custom_iam_role.example.com --zones us-test-1a
func TestCreateClusterCustomIamRole(t *testing.T) {
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/custom_iam_role", "v1alpha1")
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/custom_iam_role", "v1alpha2")
}

// TestCreateClusterHA runs kops create cluster ha.example.com --zones us-test-1a,us-test-1b,us-test-1c --master-zones us-test-1a,us-test-1b,us-test-1c
func TestCreateClusterHA(t *testing.T) {
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha", "v1alpha1")
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/kops/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ type ClusterSpec struct {
// missing: default policy (currently OS security upgrades that do not require a reboot)
UpdatePolicy *string `json:"updatePolicy,omitempty"`

// Use custom IAM roles for cluster roles
RoleCustomIamRoles map[string]string `json:"roleCustomIamRoles,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IamRoles?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to change the variable name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah ... sorry was not clear. How about the name CustomIamRoles or even IamRoles?


// Additional policies to add for roles
AdditionalPolicies *map[string]string `json:"additionalPolicies,omitempty"`

Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package kops

import (
"fmt"

"github.com/golang/glog"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/kops/v1alpha1/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ type ClusterSpec struct {
// missing: default policy (currently OS security upgrades that do not require a reboot)
UpdatePolicy *string `json:"updatePolicy,omitempty"`

// Use custom IAM roles for cluster roles
RoleCustomIamRoles map[string]string `json:"roleCustomIamRoles,omitempty"`

// Additional policies to add for roles
AdditionalPolicies *map[string]string `json:"additionalPolicies,omitempty"`

Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha1/zz_generated.conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ func autoConvert_v1alpha1_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out *
// WARNING: in.AdminAccess requires manual conversion: does not exist in peer-type
out.IsolateMasters = in.IsolateMasters
out.UpdatePolicy = in.UpdatePolicy
out.RoleCustomIamRoles = in.RoleCustomIamRoles
out.AdditionalPolicies = in.AdditionalPolicies
if in.EtcdClusters != nil {
in, out := &in.EtcdClusters, &out.EtcdClusters
Expand Down Expand Up @@ -491,6 +492,7 @@ func autoConvert_kops_ClusterSpec_To_v1alpha1_ClusterSpec(in *kops.ClusterSpec,
// WARNING: in.KubernetesAPIAccess requires manual conversion: does not exist in peer-type
out.IsolateMasters = in.IsolateMasters
out.UpdatePolicy = in.UpdatePolicy
out.RoleCustomIamRoles = in.RoleCustomIamRoles
out.AdditionalPolicies = in.AdditionalPolicies
if in.EtcdClusters != nil {
in, out := &in.EtcdClusters, &out.EtcdClusters
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/kops/v1alpha2/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ type ClusterSpec struct {
// missing: default policy (currently OS security upgrades that do not require a reboot)
UpdatePolicy *string `json:"updatePolicy,omitempty"`

// Use custom IAM roles for cluster roles
RoleCustomIamRoles map[string]string `json:"roleCustomIamRoles,omitempty"`

// Additional policies to add for roles
AdditionalPolicies *map[string]string `json:"additionalPolicies,omitempty"`

Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ func autoConvert_v1alpha2_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out *
out.KubernetesAPIAccess = in.KubernetesAPIAccess
out.IsolateMasters = in.IsolateMasters
out.UpdatePolicy = in.UpdatePolicy
out.RoleCustomIamRoles = in.RoleCustomIamRoles
out.AdditionalPolicies = in.AdditionalPolicies
if in.EtcdClusters != nil {
in, out := &in.EtcdClusters, &out.EtcdClusters
Expand Down Expand Up @@ -541,6 +542,7 @@ func autoConvert_kops_ClusterSpec_To_v1alpha2_ClusterSpec(in *kops.ClusterSpec,
out.KubernetesAPIAccess = in.KubernetesAPIAccess
out.IsolateMasters = in.IsolateMasters
out.UpdatePolicy = in.UpdatePolicy
out.RoleCustomIamRoles = in.RoleCustomIamRoles
out.AdditionalPolicies = in.AdditionalPolicies
if in.EtcdClusters != nil {
in, out := &in.EtcdClusters, &out.EtcdClusters
Expand Down
62 changes: 41 additions & 21 deletions pkg/model/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ package model
import (
"encoding/json"
"fmt"
"reflect"
"strings"
"text/template"

"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/model/iam"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
"reflect"
"strings"
"text/template"
)

// IAMModelBuilder configures IAM objects
Expand Down Expand Up @@ -64,33 +65,52 @@ func (b *IAMModelBuilder) Build(c *fi.ModelBuilderContext) error {
// Generate IAM objects etc for each role
for _, role := range roles {
name := b.IAMName(role)
roleAsString := reflect.ValueOf(role).String()

var iamRole *awstasks.IAMRole
{
rolePolicy, err := b.buildAWSIAMRolePolicy()
if err != nil {
return err
}

customIamRoles := b.Cluster.Spec.RoleCustomIamRoles

// If we've specified an IAMRoleArn for this cluster role,
// do not create a new one
if customIamRoles[roleAsString] != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Notes

I do need to have you put this functionality behind a feature flag.

arn := customIamRoles[roleAsString]
rs := strings.Split(arn, "/")
roleName := rs[len(rs)-1]
iamRole = &awstasks.IAMRole{
Name: s(name),
RolePolicyDocument: fi.WrapResource(rolePolicy),
Name: &roleName,
ID: &arn,

// We set Policy Document to nil as this role will be managed externaly
RolePolicyDocument: nil,
}
c.AddTask(iamRole)
} else {
{
rolePolicy, err := b.buildAWSIAMRolePolicy()
if err != nil {
return err
}

}
iamRole = &awstasks.IAMRole{
Name: s(name),
RolePolicyDocument: fi.WrapResource(rolePolicy),
}
c.AddTask(iamRole)
}

policy, err := b.buildAWSIAMPolicy(role)
if err != nil {
return err
}
{
t := &awstasks.IAMRolePolicy{
Name: s(name),
Role: iamRole,
PolicyDocument: fi.WrapResource(fi.NewStringResource(policy)),
policy, err := b.buildAWSIAMPolicy(role)
if err != nil {
return err
}
{
t := &awstasks.IAMRolePolicy{
Name: s(name),
Role: iamRole,
PolicyDocument: fi.WrapResource(fi.NewStringResource(policy)),
}
c.AddTask(t)
}
c.AddTask(t)
}

var iamInstanceProfile *awstasks.IAMInstanceProfile
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
apiVersion: kops/v1alpha1
kind: Cluster
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
name: custom-iam-role.example.com
spec:
adminAccess:
- 0.0.0.0/0
api:
dns: {}
channel: stable
cloudProvider: aws
configBase: memfs://tests/custom-iam-role.example.com
etcdClusters:
- etcdMembers:
- name: a
zone: us-test-1a
name: main
- etcdMembers:
- name: a
zone: us-test-1a
name: events
kubernetesVersion: v1.4.8
masterPublicName: api.custom-iam-role.example.com
networkCIDR: 172.20.0.0/16
networking:
kubenet: {}
nonMasqueradeCIDR: 100.64.0.0/10
roleCustomIamRoles:
Master: foo
Node: bar
topology:
dns:
type: Public
masters: public
nodes: public
zones:
- cidr: 172.20.32.0/19
name: us-test-1a

---

apiVersion: kops/v1alpha1
kind: InstanceGroup
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
labels:
kops.k8s.io/cluster: custom-iam-role.example.com
name: master-us-test-1a
spec:
image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
machineType: m3.medium
maxSize: 1
minSize: 1
role: Master
zones:
- us-test-1a

---

apiVersion: kops/v1alpha1
kind: InstanceGroup
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
labels:
kops.k8s.io/cluster: custom-iam-role.example.com
name: nodes
spec:
image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
machineType: t2.medium
maxSize: 2
minSize: 2
role: Node
zones:
- us-test-1a
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
apiVersion: kops/v1alpha2
kind: Cluster
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
name: custom-iam-role.example.com
spec:
api:
dns: {}
channel: stable
cloudProvider: aws
configBase: memfs://tests/custom-iam-role.example.com
etcdClusters:
- etcdMembers:
- instanceGroup: master-us-test-1a
name: a
name: main
- etcdMembers:
- instanceGroup: master-us-test-1a
name: a
name: events
kubernetesApiAccess:
- 0.0.0.0/0
kubernetesVersion: v1.4.8
masterPublicName: api.custom-iam-role.example.com
networkCIDR: 172.20.0.0/16
networking:
kubenet: {}
nonMasqueradeCIDR: 100.64.0.0/10
roleCustomIamRoles:
Master: foo
Node: bar
sshAccess:
- 0.0.0.0/0
subnets:
- cidr: 172.20.32.0/19
name: us-test-1a
type: Public
zone: us-test-1a
topology:
dns:
type: Public
masters: public
nodes: public

---

apiVersion: kops/v1alpha2
kind: InstanceGroup
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
labels:
kops.k8s.io/cluster: custom-iam-role.example.com
name: master-us-test-1a
spec:
image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
machineType: m3.medium
maxSize: 1
minSize: 1
role: Master
subnets:
- us-test-1a

---

apiVersion: kops/v1alpha2
kind: InstanceGroup
metadata:
creationTimestamp: 2017-01-01T00:00:00Z
labels:
kops.k8s.io/cluster: custom-iam-role.example.com
name: nodes
spec:
image: kope.io/k8s-1.4-debian-jessie-amd64-hvm-ebs-2016-10-21
machineType: t2.medium
maxSize: 2
minSize: 2
role: Node
subnets:
- us-test-1a
7 changes: 7 additions & 0 deletions tests/integration/create_cluster/custom_iam_role/options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ClusterName: custom-iam-role.example.com
Zones:
- us-test-1a
Cloud: aws
KubernetesVersion: v1.4.8
MasterIAMRole: foo
NodeIAMRole: bar
Loading