Merge pull request #11853 from johngmyers/override-issuer

Allow overriding the ServiceAccountIssuer for IRSA

pkg/model/awsmodel/iam.go

@@ -410,11 +410,7 @@ func (b *IAMModelBuilder) buildAWSIAMRolePolicy(role iam.Subject) (fi.Resource,
 	var policy string
 	serviceAccount, ok := role.ServiceAccount()
 	if ok {
-		serviceAccountIssuer, err := iam.ServiceAccountIssuer(&b.Cluster.Spec)
+		oidcProvider := strings.TrimPrefix(*b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer, "https://")
-		if err != nil {
-			return nil, err
-		}
-		oidcProvider := strings.TrimPrefix(serviceAccountIssuer, "https://")
 
 		iamPolicy := &iam.Policy{
 			Version: iam.PolicyDefaultVersion,

pkg/model/awsmodel/oidc_provider.go

@@ -17,7 +17,6 @@ limitations under the License.
 package awsmodel
 
 import (
-	"k8s.io/kops/pkg/model/iam"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
 )
@@ -42,11 +41,6 @@ func (b *OIDCProviderBuilder) Build(c *fi.ModelBuilderContext) error {
 		return nil
 	}
 
-	serviceAccountIssuer, err := iam.ServiceAccountIssuer(&b.Cluster.Spec)
-	if err != nil {
-		return err
-	}
-
 	fingerprints := getFingerprints()
 
 	thumbprints := []*string{}
@@ -58,7 +52,7 @@ func (b *OIDCProviderBuilder) Build(c *fi.ModelBuilderContext) error {
 	c.AddTask(&awstasks.IAMOIDCProvider{
 		Name:        fi.String(b.ClusterName()),
 		Lifecycle:   b.Lifecycle,
-		URL:         fi.String(serviceAccountIssuer),
+		URL:         b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer,
 		ClientIDs:   []*string{fi.String(defaultAudience)},
 		Tags:        b.CloudTags(b.ClusterName(), false),
 		Thumbprints: thumbprints,

pkg/model/components/BUILD.bazel

@@ -32,8 +32,8 @@ go_library(
         "//pkg/apis/kops:go_default_library",
         "//pkg/apis/kops/util:go_default_library",
         "//pkg/assets:go_default_library",
+        "//pkg/dns:go_default_library",
         "//pkg/k8sversion:go_default_library",
-        "//pkg/model/iam:go_default_library",
         "//pkg/wellknownports:go_default_library",
         "//upup/pkg/fi:go_default_library",
         "//upup/pkg/fi/cloudup/gce:go_default_library",

pkg/model/components/discovery.go

@@ -17,10 +17,14 @@ limitations under the License.
 package components
 
 import (
+	"fmt"
+	"strings"
+
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/model/iam"
+	"k8s.io/kops/pkg/dns"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/loader"
+	"k8s.io/kops/util/pkg/vfs"
 )
 
 // DiscoveryOptionsBuilder adds options for identity discovery to the model (mostly kube-apiserver)
@@ -33,6 +37,10 @@ var _ loader.OptionsBuilder = &DiscoveryOptionsBuilder{}
 func (b *DiscoveryOptionsBuilder) BuildOptions(o interface{}) error {
 	clusterSpec := o.(*kops.ClusterSpec)
 
+	if clusterSpec.KubeAPIServer == nil {
+		clusterSpec.KubeAPIServer = &kops.KubeAPIServerConfig{}
+	}
+
 	if b.IsKubernetesLT("1.20") {
 		if clusterSpec.KubeAPIServer.FeatureGates == nil {
 			return nil
@@ -42,31 +50,61 @@ func (b *DiscoveryOptionsBuilder) BuildOptions(o interface{}) error {
 		}
 	}
 
-	if clusterSpec.KubeAPIServer == nil {
-		clusterSpec.KubeAPIServer = &kops.KubeAPIServerConfig{}
-	}
-
 	kubeAPIServer := clusterSpec.KubeAPIServer
 
 	if len(kubeAPIServer.APIAudiences) == 0 {
 		kubeAPIServer.APIAudiences = []string{"kubernetes.svc.default"}
 	}
 
-	serviceAccountIssuer, err := iam.ServiceAccountIssuer(clusterSpec)
+	if kubeAPIServer.ServiceAccountIssuer == nil {
-	if err != nil {
+		said := clusterSpec.ServiceAccountIssuerDiscovery
-		return err
+		var serviceAccountIssuer string
-	}
+		if said != nil && said.DiscoveryStore != "" {
-	kubeAPIServer.ServiceAccountIssuer = &serviceAccountIssuer
+			store := said.DiscoveryStore
-
+			base, err := vfs.Context.BuildVfsPath(store)
-	// We set apiserver ServiceAccountKey and ServiceAccountSigningKeyFile in nodeup
+			if err != nil {
-
+				return fmt.Errorf("error parsing locationStore=%q: %w", store, err)
-	if kubeAPIServer.ServiceAccountJWKSURI == nil {
+			}
-		jwksURI, err := iam.ServiceAccountIssuer(clusterSpec)
+			switch base := base.(type) {
-		if err != nil {
+			case *vfs.S3Path:
-			return err
+				serviceAccountIssuer, err = base.GetHTTPsUrl()
+				if err != nil {
+					return err
+				}
+			case *vfs.MemFSPath:
+				if !base.IsClusterReadable() {
+					// If this _is_ a test, we should call MarkClusterReadable
+					return fmt.Errorf("locationStore=%q is only supported in tests", store)
+				}
+				serviceAccountIssuer = strings.Replace(base.Path(), "memfs://", "https://", 1)
+			default:
+				return fmt.Errorf("locationStore=%q is of unexpected type %T", store, base)
+			}
+		} else {
+			if dns.IsGossipHostname(clusterSpec.MasterInternalName) {
+				serviceAccountIssuer = "https://kubernetes.default"
+			} else if supportsPublicJWKS(clusterSpec) {
+				serviceAccountIssuer = "https://" + clusterSpec.MasterPublicName
+			} else {
+				serviceAccountIssuer = "https://" + clusterSpec.MasterInternalName
+			}
 		}
-		kubeAPIServer.ServiceAccountJWKSURI = fi.String(jwksURI + "/openid/v1/jwks")
+		kubeAPIServer.ServiceAccountIssuer = &serviceAccountIssuer
 	}
+	kubeAPIServer.ServiceAccountJWKSURI = fi.String(*kubeAPIServer.ServiceAccountIssuer + "/openid/v1/jwks")
+	// We set apiserver ServiceAccountKey and ServiceAccountSigningKeyFile in nodeup
 
 	return nil
 }
+
+func supportsPublicJWKS(clusterSpec *kops.ClusterSpec) bool {
+	if !fi.BoolValue(clusterSpec.KubeAPIServer.AnonymousAuth) {
+		return false
+	}
+	for _, cidr := range clusterSpec.KubernetesAPIAccess {
+		if cidr == "0.0.0.0/0" || cidr == "::/0" {
+			return true
+		}
+	}
+	return false
+}

pkg/model/iam/BUILD.bazel

@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//pkg/apis/kops:go_default_library",
         "//pkg/apis/kops/model:go_default_library",
-        "//pkg/dns:go_default_library",
         "//pkg/util/stringorslice:go_default_library",
         "//pkg/wellknownusers:go_default_library",
         "//upup/pkg/fi:go_default_library",

pkg/model/iam/subject.go

@@ -18,15 +18,11 @@ package iam
 
 import (
 	"fmt"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/dns"
 	"k8s.io/kops/pkg/wellknownusers"
-	"k8s.io/kops/upup/pkg/fi"
-	"k8s.io/kops/util/pkg/vfs"
 )
 
 // Subject represents an IAM identity, to which permissions are granted.
@@ -111,50 +107,6 @@ func BuildNodeRoleSubject(igRole kops.InstanceGroupRole, enableLifecycleHookPerm
 	}
 }
 
-// ServiceAccountIssuer determines the issuer in the ServiceAccount JWTs
-func ServiceAccountIssuer(clusterSpec *kops.ClusterSpec) (string, error) {
-	said := clusterSpec.ServiceAccountIssuerDiscovery
-	if said != nil && said.DiscoveryStore != "" {
-		store := said.DiscoveryStore
-		base, err := vfs.Context.BuildVfsPath(store)
-		if err != nil {
-			return "", fmt.Errorf("error parsing locationStore=%q: %w", store, err)
-		}
-		switch base := base.(type) {
-		case *vfs.S3Path:
-			return base.GetHTTPsUrl()
-		case *vfs.MemFSPath:
-			if !base.IsClusterReadable() {
-				// If this _is_ a test, we should call MarkClusterReadable
-				return "", fmt.Errorf("locationStore=%q is only supported in tests", store)
-			}
-			return strings.Replace(base.Path(), "memfs://", "https://", 1), nil
-		default:
-			return "", fmt.Errorf("locationStore=%q is of unexpected type %T", store, base)
-		}
-	} else {
-		if dns.IsGossipHostname(clusterSpec.MasterInternalName) {
-			return "https://kubernetes.default", nil
-		}
-		if supportsPublicJWKS(clusterSpec) {
-			return "https://" + clusterSpec.MasterPublicName, nil
-		}
-		return "https://" + clusterSpec.MasterInternalName, nil
-	}
-}
-
-func supportsPublicJWKS(clusterSpec *kops.ClusterSpec) bool {
-	if !fi.BoolValue(clusterSpec.KubeAPIServer.AnonymousAuth) {
-		return false
-	}
-	for _, cidr := range clusterSpec.KubernetesAPIAccess {
-		if cidr == "0.0.0.0/0" || cidr == "::/0" {
-			return true
-		}
-	}
-	return false
-}
-
 // AddServiceAccountRole adds the appropriate mounts / env vars to enable a pod to use a service-account role
 func AddServiceAccountRole(context *IAMModelContext, podSpec *corev1.PodSpec, serviceAccountRole Subject) error {
 	cloudProvider := kops.CloudProviderID(context.Cluster.Spec.CloudProvider)

pkg/model/issuerdiscovery.go

@@ -28,7 +28,6 @@ import (
 
 	"gopkg.in/square/go-jose.v2"
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/model/iam"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/fitasks"
 )
@@ -68,12 +67,7 @@ func (b *IssuerDiscoveryModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		SigningKey: skTask,
 	}
 
-	serviceAccountIssuer, err := iam.ServiceAccountIssuer(&b.Cluster.Spec)
+	discovery, err := buildDiscoveryJSON(*b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer)
-	if err != nil {
-		return err
-	}
-
-	discovery, err := buildDiscoveryJSON(serviceAccountIssuer)
 	if err != nil {
 		return err
 	}

upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go

@@ -950,7 +950,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
 		})
 	}
 
-	if kops.CloudProviderID(b.Cluster.Spec.CloudProvider) == kops.CloudProviderAWS {
+	if kops.CloudProviderID(b.Cluster.Spec.CloudProvider) == kops.CloudProviderAWS && b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer != nil {
 		awsModelContext := &awsmodel.AWSModelContext{
 			KopsModelContext: b.KopsModelContext,
 		}

upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/cluster.yaml

@@ -21,7 +21,7 @@ spec:
       name: master-us-test-1a
     name: events
   iam: {}
-  kubernetesVersion: v1.14.6
+  kubernetesVersion: v1.20.6
   masterInternalName: api.internal.minimal.example.com
   masterPublicName: api.minimal.example.com
   additionalSans:

upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/kops-controller.addons.k8s.io-k8s-1.16.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 data:
   config.yaml: |
-    {"cloud":"aws","configBase":"memfs://clusters.example.com/minimal.example.com"}
+    {"cloud":"aws","configBase":"memfs://clusters.example.com/minimal.example.com","server":{"Listen":":3988","provider":{"aws":{"nodesRoles":["kops-custom-node-role","nodes.minimal.example.com"],"Region":"us-east-1"}},"serverKeyPath":"/etc/kubernetes/kops-controller/pki/kops-controller.key","serverCertificatePath":"/etc/kubernetes/kops-controller/pki/kops-controller.crt","caBasePath":"/etc/kubernetes/kops-controller/pki","signingCAs":["ca"],"certNames":["kubelet","kubelet-server","kube-proxy"]}}
 kind: ConfigMap
 metadata:
   creationTimestamp: null
@@ -32,6 +32,8 @@ spec:
       k8s-app: kops-controller
   template:
     metadata:
+      annotations:
+        dns.alpha.kubernetes.io/internal: kops-controller.internal.minimal.example.com
       labels:
         k8s-addon: kops-controller.addons.k8s.io
         k8s-app: kops-controller

upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/manifest.yaml

@@ -6,7 +6,7 @@ spec:
   addons:
   - id: k8s-1.16
     manifest: kops-controller.addons.k8s.io/k8s-1.16.yaml
-    manifestHash: 40ffaead818a21b374588ed2f94b517b9569c71f
+    manifestHash: 90b1206425c7a034147e2373ea00193018a1fb4a
     name: kops-controller.addons.k8s.io
     needsRollingUpdate: control-plane
     selector:
@@ -17,17 +17,11 @@ spec:
     selector:
       k8s-addon: core.addons.k8s.io
   - id: k8s-1.12
-    manifest: kube-dns.addons.k8s.io/k8s-1.12.yaml
+    manifest: coredns.addons.k8s.io/k8s-1.12.yaml
-    manifestHash: 470684b80af41aa53c25bc5ac1a78b259c93336b
+    manifestHash: 004bda4e250d9cec5d5f3e732056020b78b0ab88
-    name: kube-dns.addons.k8s.io
+    name: coredns.addons.k8s.io
     selector:
-      k8s-addon: kube-dns.addons.k8s.io
+      k8s-addon: coredns.addons.k8s.io
-  - id: k8s-1.8
-    manifest: rbac.addons.k8s.io/k8s-1.8.yaml
-    manifestHash: 38ba4e0078bb1225421114f76f121c2277ca92ac
-    name: rbac.addons.k8s.io
-    selector:
-      k8s-addon: rbac.addons.k8s.io
   - id: k8s-1.9
     manifest: kubelet-api.rbac.addons.k8s.io/k8s-1.9.yaml
     manifestHash: 8ee090e41be5e8bcd29ee799b1608edcd2dd8b65