diff --git a/pkg/model/awsmodel/iam.go b/pkg/model/awsmodel/iam.go index 319c9d8a4c..4495b56faa 100644 --- a/pkg/model/awsmodel/iam.go +++ b/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) - if err != nil { - return nil, err - } - oidcProvider := strings.TrimPrefix(serviceAccountIssuer, "https://") + oidcProvider := strings.TrimPrefix(*b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer, "https://") iamPolicy := &iam.Policy{ Version: iam.PolicyDefaultVersion, diff --git a/pkg/model/awsmodel/oidc_provider.go b/pkg/model/awsmodel/oidc_provider.go index 063c71c829..bdc2833e11 100644 --- a/pkg/model/awsmodel/oidc_provider.go +++ b/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, diff --git a/pkg/model/components/BUILD.bazel b/pkg/model/components/BUILD.bazel index 4e75f19e71..07c7b68002 100644 --- a/pkg/model/components/BUILD.bazel +++ b/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", diff --git a/pkg/model/components/discovery.go b/pkg/model/components/discovery.go index 5cf139d2aa..70d6d4d201 100644 --- a/pkg/model/components/discovery.go +++ b/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 err != nil { - return err - } - kubeAPIServer.ServiceAccountIssuer = &serviceAccountIssuer - - // We set apiserver ServiceAccountKey and ServiceAccountSigningKeyFile in nodeup - - if kubeAPIServer.ServiceAccountJWKSURI == nil { - jwksURI, err := iam.ServiceAccountIssuer(clusterSpec) - if err != nil { - return err + if kubeAPIServer.ServiceAccountIssuer == nil { + said := clusterSpec.ServiceAccountIssuerDiscovery + var serviceAccountIssuer string + 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: + 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 +} diff --git a/pkg/model/iam/BUILD.bazel b/pkg/model/iam/BUILD.bazel index 92914588b2..8715cdc2d4 100644 --- a/pkg/model/iam/BUILD.bazel +++ b/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", diff --git a/pkg/model/iam/subject.go b/pkg/model/iam/subject.go index b6204957e2..992bf64b9b 100644 --- a/pkg/model/iam/subject.go +++ b/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) diff --git a/pkg/model/issuerdiscovery.go b/pkg/model/issuerdiscovery.go index 48f6c42c57..24b2039eb2 100644 --- a/pkg/model/issuerdiscovery.go +++ b/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) - if err != nil { - return err - } - - discovery, err := buildDiscoveryJSON(serviceAccountIssuer) + discovery, err := buildDiscoveryJSON(*b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer) if err != nil { return err } diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go index e09767c921..f5477124ad 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go +++ b/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, } diff --git a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/cluster.yaml b/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/cluster.yaml index 41da76d95e..527e61d85b 100644 --- a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/cluster.yaml +++ b/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: diff --git a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/kops-controller.addons.k8s.io-k8s-1.16.yaml b/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/kops-controller.addons.k8s.io-k8s-1.16.yaml index 581d6270cc..84f544e0e3 100644 --- a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/kops-controller.addons.k8s.io-k8s-1.16.yaml +++ b/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 diff --git a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/manifest.yaml b/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/manifest.yaml index 0a77fde01a..b3f9cf6f42 100644 --- a/upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/service-account-iam/manifest.yaml +++ b/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 - manifestHash: 470684b80af41aa53c25bc5ac1a78b259c93336b - name: kube-dns.addons.k8s.io + manifest: coredns.addons.k8s.io/k8s-1.12.yaml + manifestHash: 004bda4e250d9cec5d5f3e732056020b78b0ab88 + name: coredns.addons.k8s.io selector: - k8s-addon: kube-dns.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 + k8s-addon: coredns.addons.k8s.io - id: k8s-1.9 manifest: kubelet-api.rbac.addons.k8s.io/k8s-1.9.yaml manifestHash: 8ee090e41be5e8bcd29ee799b1608edcd2dd8b65