Skip to content

Commit

Permalink
Remove UseServiceAccountIAM feature flag and rename feature to UseSer…
Browse files Browse the repository at this point in the history
…viceAccountExternalPermissions
  • Loading branch information
Ole Markus With committed Aug 7, 2021
1 parent 82e4e5a commit 377f549
Show file tree
Hide file tree
Showing 28 changed files with 81 additions and 55 deletions.
16 changes: 2 additions & 14 deletions cmd/kops/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,14 +392,8 @@ func TestPrivateDns2(t *testing.T) {
runTestTerraformAWS(t)
}

// TestDiscoveryFeatureGate runs a simple configuration, but with UseServiceAccountIAM and the ServiceAccountIssuerDiscovery feature gate enabled
// TestDiscoveryFeatureGate runs a simple configuration, but with UseServiceAccountExternalPermissions and the ServiceAccountIssuerDiscovery feature gate enabled
func TestDiscoveryFeatureGate(t *testing.T) {
featureflag.ParseFlags("+UseServiceAccountIAM")
unsetFeatureFlags := func() {
featureflag.ParseFlags("-UseServiceAccountIAM")
}
defer unsetFeatureFlags()

newIntegrationTest("minimal.example.com", "public-jwks-apiserver").
withServiceAccountRole("dns-controller.kube-system", true).
withOIDCDiscovery().
Expand All @@ -416,14 +410,8 @@ func TestVFSServiceAccountIssuerDiscovery(t *testing.T) {

}

// TestAWSLBController runs a simple configuration, but with AWS LB controller and UseServiceAccountIAM enabled
// TestAWSLBController runs a simple configuration, but with AWS LB controller and UseServiceAccountExternalPermissions enabled
func TestAWSLBController(t *testing.T) {
featureflag.ParseFlags("+UseServiceAccountIAM")
unsetFeatureFlags := func() {
featureflag.ParseFlags("-UseServiceAccountIAM")
}
defer unsetFeatureFlags()

newIntegrationTest("minimal.example.com", "aws-lb-controller").
withOIDCDiscovery().
withServiceAccountRole("dns-controller.kube-system", true).
Expand Down
14 changes: 14 additions & 0 deletions docs/releases/1.22-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ spec:
httpTokens: optional
```

## External ServiceAccountPermissions

Many of kOps addons can now make direct use of external permissions.
This can be enabled by adding the following to the Cluster spec:

```
spec:
iam:
useServiceAcountExternalPermissions: true
```

Currently this is only available using the AWS cloud provider.


## Other significant changes

* New clusters on AWS will no longer provision an SSH public key by default. To provision
Expand Down
6 changes: 6 additions & 0 deletions k8s/crds/kops.k8s.io_clusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,12 @@ spec:
- namespace
type: object
type: array
useServiceAccountExternalPermissions:
description: UseServiceAccountExternalPermissions determines if
managed ServiceAccounts will use external permissions directly.
If this is set to false, ServiceAccounts will assume external
permissions from the instances they run on.
type: boolean
required:
- legacy
type: object
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 @@ -304,6 +304,9 @@ type IAMSpec struct {
Legacy bool `json:"legacy"`
AllowContainerRegistry bool `json:"allowContainerRegistry,omitempty"`
PermissionsBoundary *string `json:"permissionsBoundary,omitempty"`
// UseServiceAccountExternalPermissions determines if managed ServiceAccounts will use external permissions directly.
// If this is set to false, ServiceAccounts will assume external permissions from the instances they run on.
UseServiceAccountExternalPermissions *bool `json:"useServiceAccountExternalPermissions,omitempty"`
// ServiceAccountExternalPermissions defines the relatinship between Kubernetes ServiceAccounts and permissions with external resources.
ServiceAccountExternalPermissions []ServiceAccountExternalPermission `json:"serviceAccountExternalPermissions,omitempty"`
}
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 @@ -301,6 +301,9 @@ type IAMSpec struct {
Legacy bool `json:"legacy"`
AllowContainerRegistry bool `json:"allowContainerRegistry,omitempty"`
PermissionsBoundary *string `json:"permissionsBoundary,omitempty"`
// UseServiceAccountExternalPermissions determines if managed ServiceAccounts will use external permissions directly.
// If this is set to false, ServiceAccounts will assume external permissions from the instances they run on.
UseServiceAccountExternalPermissions *bool `json:"useServiceAccountExternalPermissions,omitempty"`
// ServiceAccountExternalPermissions defines the relatinship between Kubernetes ServiceAccounts and permissions with external resources.
ServiceAccountExternalPermissions []ServiceAccountExternalPermission `json:"serviceAccountExternalPermissions,omitempty"`
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/kops/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions pkg/featureflag/featureflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ var (
TerraformJSON = new("TerraformJSON", Bool(false))
// ClusterAddons activates experimental cluster-addons support
ClusterAddons = new("ClusterAddons", Bool(false))
// UseServiceAccountIAM controls whether we use pod-level IAM permissions for our system pods and kOps addons.
UseServiceAccountIAM = new("UseServiceAccountIAM", Bool(false))
// Azure toggles the Azure support.
Azure = new("Azure", Bool(false))
// KopsControllerStateStore enables fetching the kops state from kops-controller, instead of requiring access to S3/GCS/etc.
Expand Down
1 change: 0 additions & 1 deletion pkg/model/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pkg/model/awsmodel/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ func (b *IAMModelBuilder) buildIAMRole(role iam.Subject, iamName string, c *fi.M
func (b *IAMModelBuilder) buildIAMRolePolicy(role iam.Subject, iamName string, iamRole *awstasks.IAMRole, c *fi.ModelBuilderContext) error {
iamPolicy := &iam.PolicyResource{
Builder: &iam.PolicyBuilder{
Cluster: b.Cluster,
Role: role,
Region: b.Region,
UseServiceAccountIAM: b.UseServiceAccountIAM(),
Cluster: b.Cluster,
Role: role,
Region: b.Region,
UseServiceAccountExternalPermisssions: b.UseServiceAccountExternalPermissions(),
},
}

Expand Down
1 change: 0 additions & 1 deletion pkg/model/components/addonmanifests/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/model/components/addonmanifests/dnscontroller/remap.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

// Remap remaps the dns-controller addon
func Remap(context *model.KopsModelContext, addon *addonsapi.AddonSpec, objects []*kubemanifest.Object) error {
if !context.UseServiceAccountIAM() {
if !context.UseServiceAccountExternalPermissions() {
return nil
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/model/components/addonmanifests/remap.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"k8s.io/klog/v2"
addonsapi "k8s.io/kops/channels/pkg/api"
"k8s.io/kops/pkg/assets"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/kubemanifest"
"k8s.io/kops/pkg/model"
"k8s.io/kops/pkg/model/components/addonmanifests/awscloudcontrollermanager"
Expand Down Expand Up @@ -83,7 +82,7 @@ func RemapAddonManifest(addon *addonsapi.AddonSpec, context *model.KopsModelCont
}

func addServiceAccountRole(context *model.KopsModelContext, objects kubemanifest.ObjectList) error {
if !featureflag.UseServiceAccountIAM.Enabled() {
if !context.UseServiceAccountExternalPermissions() {
return nil
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/model"
"k8s.io/kops/pkg/apis/kops/util"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/model/components"
"k8s.io/kops/pkg/model/iam"
nodeidentityaws "k8s.io/kops/pkg/nodeidentity/aws"
Expand Down Expand Up @@ -414,7 +413,9 @@ func (b *KopsModelContext) NodePortRange() (utilnet.PortRange, error) {
return defaultServiceNodePortRange, nil
}

// UseServiceAccountIAM returns true if we are using service-account bound IAM roles.
func (b *KopsModelContext) UseServiceAccountIAM() bool {
return featureflag.UseServiceAccountIAM.Enabled()
// UseServiceAccountExternalPermissions returns true if we are using service-account bound IAM roles.
func (b *KopsModelContext) UseServiceAccountExternalPermissions() bool {

return b.Cluster.Spec.IAM != nil &&
fi.BoolValue(b.Cluster.Spec.IAM.UseServiceAccountExternalPermissions)
}
18 changes: 9 additions & 9 deletions pkg/model/iam/iam_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,13 +230,13 @@ func (l *Statement) Equal(r *Statement) bool {
// PolicyBuilder struct defines all valid fields to be used when building the
// AWS IAM policy document for a given instance group role.
type PolicyBuilder struct {
Cluster *kops.Cluster
HostedZoneID string
KMSKeys []string
Region string
ResourceARN *string
Role Subject
UseServiceAccountIAM bool
Cluster *kops.Cluster
HostedZoneID string
KMSKeys []string
Region string
ResourceARN *string
Role Subject
UseServiceAccountExternalPermisssions bool
}

// BuildAWSPolicy builds a set of IAM policy statements based on the
Expand Down Expand Up @@ -325,7 +325,7 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
addKMSIAMPolicies(p, stringorslice.Slice(b.KMSKeys))
}

// Protokube needs dns-controller permissions in instance role even if UseServiceAccountIAM.
// Protokube needs dns-controller permissions in instance role even if UseServiceAccountExternalPermissions.
AddDNSControllerPermissions(b, p)

// If cluster does not use external CCM, the master IAM Role needs CCM permissions
Expand All @@ -334,7 +334,7 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
AddLegacyCCMPermissions(p)
}

if !b.UseServiceAccountIAM {
if !b.UseServiceAccountExternalPermisssions {
esc := b.Cluster.Spec.SnapshotController != nil &&
fi.BoolValue(b.Cluster.Spec.SnapshotController.Enabled)
AddAWSEBSCSIDriverPermissions(p, esc)
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/scenarios/lib/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ fi
if [[ ${KOPS_IRSA-} = true ]]; then
OVERRIDES="${OVERRIDES-} --override=cluster.spec.serviceAccountIssuerDiscovery.discoveryStore=${DISCOVERY_STORE}/${CLUSTER_NAME}/discovery"
OVERRIDES="${OVERRIDES} --override=cluster.spec.serviceAccountIssuerDiscovery.enableAWSOIDCProvider=true"
KOPS_FEATURE_FLAGS="UseServiceAccountIAM,${KOPS_FEATURE_FLAGS}"
fi

export GO111MODULE=on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ spec:
version: 3.4.13
iam:
legacy: false
useServiceAccountExternalPermissions: true
keyStore: memfs://clusters.example.com/minimal.example.com/pki
kubeAPIServer:
allowPrivileged: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ spec:
- instanceGroup: master-us-test-1a
name: us-test-1a
name: events
iam: {}
iam:
useServiceAccountExternalPermissions: true
kubelet:
anonymousAuth: false
kubernetesVersion: v1.21.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ spec:
version: 3.4.13
iam:
legacy: false
useServiceAccountExternalPermissions: true
keyStore: memfs://clusters.example.com/minimal.example.com/pki
kubeAPIServer:
allowPrivileged: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ spec:
- instanceGroup: master-us-test-1a
name: us-test-1a
name: events
iam: {}
iam:
useServiceAccountExternalPermissions: true
kubeAPIServer:
featureGates:
ServiceAccountIssuerDiscovery: "true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,12 +415,12 @@ spec:
topologyKey: topology.kubernetes.io/zone
nodeSelector:
kubernetes.io/os: linux
{{ if not UseServiceAccountIAM }}
{{ if not UseServiceAccountExternalPermissions }}
node-role.kubernetes.io/master: ""
{{ end }}
serviceAccountName: ebs-csi-controller-sa
priorityClassName: system-cluster-critical
{{ if not UseServiceAccountIAM }}
{{ if not UseServiceAccountExternalPermissions }}
tolerations:
- operator: Exists
{{ end }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ spec:
topologyKey: topology.kubernetes.io/zone
priorityClassName: system-cluster-critical
serviceAccountName: cluster-autoscaler
{{ if not UseServiceAccountIAM }}
{{ if not UseServiceAccountExternalPermissions }}
tolerations:
- operator: "Exists"
key: node-role.kubernetes.io/master
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ spec:
requests:
cpu: {{ .CPURequest }}
memory: {{ .MemoryRequest }}
{{ if not UseServiceAccountIAM }}
{{ if not UseServiceAccountExternalPermissions }}
nodeSelector:
node-role.kubernetes.io/master: ""
tolerations:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
}

// Generate dns-controller ServiceAccount IAM permissions
if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &dnscontroller.ServiceAccount{})
}
}
Expand Down Expand Up @@ -502,7 +502,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
}
}

if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &clusterautoscaler.ServiceAccount{})
}

Expand Down Expand Up @@ -562,7 +562,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
})
}

if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &nodeterminationhandler.ServiceAccount{})
}
}
Expand Down Expand Up @@ -604,7 +604,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
}

// Generate aws-load-balancer-controller ServiceAccount IAM permissions
if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &awsloadbalancercontroller.ServiceAccount{})
}
}
Expand Down Expand Up @@ -906,7 +906,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
Id: id,
})
}
if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &awscloudcontrollermanager.ServiceAccount{})
}
}
Expand All @@ -925,7 +925,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
}

// Generate aws-load-balancer-controller ServiceAccount IAM permissions
if b.UseServiceAccountIAM() {
if b.UseServiceAccountExternalPermissions() {
serviceAccountRoles = append(serviceAccountRoles, &awsebscsidriver.ServiceAccount{})
}
}
Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ func TestBootstrapChannelBuilder_ServiceAccountIAM(t *testing.T) {

h.SetupMockAWS()

featureflag.ParseFlags("+UseServiceAccountIAM")
featureflag.ParseFlags("+UseServiceAccountExternalPermissions")
unsetFeatureFlag := func() {
featureflag.ParseFlags("-UseServiceAccountIAM")
featureflag.ParseFlags("-UseServiceAccountExternalPermissions")
}
defer unsetFeatureFlag()
runChannelBuilderTest(t, "service-account-iam", []string{"dns-controller.addons.k8s.io-k8s-1.12", "kops-controller.addons.k8s.io-k8s-1.16"})
Expand Down
Loading

0 comments on commit 377f549

Please sign in to comment.