Auto-discover caBundle when missing (take 2)

cmd/controller/main.go

@@ -26,6 +26,7 @@ import (
 	"github.com/awslabs/karpenter/pkg/controllers/allocation"
 	"github.com/awslabs/karpenter/pkg/controllers/node"
 	"github.com/awslabs/karpenter/pkg/controllers/termination"
+	"github.com/awslabs/karpenter/pkg/utils/restconfig"
 	"github.com/go-logr/zapr"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
@@ -66,10 +67,14 @@ func main() {
 	config := controllerruntime.GetConfigOrDie()
 	clientSet := kubernetes.NewForConfigOrDie(config)
 
-	// 1. Setup logger and watch for changes to log level
+	// 1. Set up logger and watch for changes to log level
 	ctx := LoggingContextOrDie(config, clientSet)
 
-	// 2. Setup controller runtime controller
+	// 2. Put REST config in context, as it can be used by arbitrary
+	// parts of the code base
+	ctx = restconfig.Inject(ctx, config)
+
+	// 3. Set up controller runtime controller
 	cloudProvider := registry.NewCloudProvider(ctx, cloudprovider.Options{ClientSet: clientSet})
 	manager := controllers.NewManagerOrDie(config, controllerruntime.Options{
 		Logger:                 zapr.NewLogger(logging.FromContext(ctx).Desugar()),

pkg/apis/provisioning/v1alpha3/provisioner_defaults.go

@@ -17,65 +17,43 @@ package v1alpha3
 import (
 	"context"
 	"encoding/base64"
-	"errors"
-	"io/ioutil"
-	"os"
-)
 
-const (
-	InClusterCABundlePath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
+	"github.com/awslabs/karpenter/pkg/utils/restconfig"
+	"k8s.io/client-go/transport"
+	"knative.dev/pkg/logging"
+	"knative.dev/pkg/ptr"
 )
 
 // SetDefaults for the provisioner
 func (p *Provisioner) SetDefaults(ctx context.Context) {}
 
-// SetDefaults for the provisioner, cascading to all subspecs
-func (s *ProvisionerSpec) SetDefaults(ctx context.Context) {}
-
-// WithDefaults returns a copy of this Provisioner with some empty/missing
-// properties replaced by (potentially dynamic) cloud provider agnostic default values.
-// The returned copy might be complemented by dynamic default values which
-// must not be hoisted (saved) into the original Provisioner CRD as those
-// default values might change over time (e.g. rolling upgrade of CABundle, ...).
-func (p *Provisioner) WithDynamicDefaults(ctx context.Context) (_ Provisioner, err error) {
-	provisioner := *p.DeepCopy()
-	provisioner.Spec, err = provisioner.Spec.withDynamicDefaults()
-	return provisioner, err
-}
-
-// WithDefaults returns a copy of this ProvisionerSpec with some empty
-// properties replaced by default values.
-func (s *ProvisionerSpec) withDynamicDefaults() (_ ProvisionerSpec, err error) {
-	spec := *s.DeepCopy()
-	spec.Cluster, err = spec.Cluster.withDynamicDefaults()
-	return spec, err
-}
-
-// WithDefaults returns a copy of this Cluster with some empty
-// properties replaced by default values. Notably, it will try
-// to load the CABundle from the in-cluster configuraiton if it
-// is not explicitly set.
-func (c *Cluster) withDynamicDefaults() (_ Cluster, err error) {
-	cluster := *c.DeepCopy()
-	cluster.CABundle, err = cluster.getCABundle()
-	return cluster, err
-}
-
-func (c *Cluster) getCABundle() (*string, error) {
+func (c *Cluster) GetCABundle(ctx context.Context) (*string, error) {
 	if c.CABundle != nil {
-		// If CABundle is explicitly provided use that one. An empty string is
-		// a valid value here if the intention is to disable the in-cluster CABundle
-		// and using the HTTP client's default trust-store (CABundle) instead.
+		// If CABundle is explicitly provided, use that one. An empty
+		// string is a valid value here if the intention is to disable
+		// the in-cluster CABundle, and using the HTTP client's
+		// default trust-store (CABundle) instead.
 		return c.CABundle, nil
 	}
-	// Otherwise, fallback to the in-cluster configuration.
-	binary, err := ioutil.ReadFile(InClusterCABundlePath)
+
+	// Discover CA Bundle from the REST client. We could alternatively
+	// have used the simpler client-go InClusterConfig() method.
+	// However, that only works when Karpenter is running as a Pod
+	// within the same cluster it's managing.
+	restConfig := restconfig.Get(ctx)
+	if restConfig == nil {
+		return nil, nil
+	}
+	transportConfig, err := restConfig.TransportConfig()
+	if err != nil {
+		logging.FromContext(ctx).Debugf("Unable to discover caBundle, loading transport config, %v", err)
+		return nil, err
+	}
+	_, err = transport.TLSConfigFor(transportConfig) // fills in CAData!
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return nil, nil
-		}
+		logging.FromContext(ctx).Debugf("Unable to discover caBundle, loading TLS config, %v", err)
 		return nil, err
 	}
-	encoded := base64.StdEncoding.EncodeToString(binary)
-	return &encoded, nil
+	logging.FromContext(ctx).Debugf("Discovered caBundle, length %d", len(transportConfig.TLS.CAData))
+	return ptr.String(base64.StdEncoding.EncodeToString(transportConfig.TLS.CAData)), nil
 }

pkg/cloudprovider/aws/cloudprovider.go

@@ -33,6 +33,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
+	"knative.dev/pkg/ptr"
 )
 
 const (
@@ -174,7 +175,7 @@ func (c *CloudProvider) ValidateConstraints(ctx context.Context, constraints *v1
 
 // Validate cloud provider specific components of the cluster spec.
 func (c *CloudProvider) ValidateSpec(ctx context.Context, spec *v1alpha3.ProvisionerSpec) (errs *apis.FieldError) {
-	if spec.Cluster.Name == nil || len(*spec.Cluster.Name) == 0 {
+	if ptr.StringValue(spec.Cluster.Name) == "" {
 		errs = errs.Also(apis.ErrMissingField("name")).ViaField("cluster")
 	}
 	return errs

pkg/cloudprovider/aws/launchtemplate.go

@@ -19,7 +19,6 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
-	"text/template"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
@@ -35,18 +34,6 @@ import (
 
 const (
 	launchTemplateNameFormat = "Karpenter-%s-%s"
-	bottlerocketUserData     = `
-[settings.kubernetes]
-api-server = "{{.Cluster.Endpoint}}"
-{{if .Cluster.CABundle}}{{if len .Cluster.CABundle}}cluster-certificate = "{{.Cluster.CABundle}}"{{end}}{{end}}
-cluster-name = "{{if .Cluster.Name}}{{.Cluster.Name}}{{end}}"
-{{if .Constraints.Labels }}[settings.kubernetes.node-labels]{{ end }}
-{{ range $Key, $Value := .Constraints.Labels }}"{{ $Key }}" = "{{ $Value }}"
-{{ end }}
-{{if .Constraints.Taints }}[settings.kubernetes.node-taints]{{ end }}
-{{ range $Taint := .Constraints.Taints }}"{{ $Taint.Key }}" = "{{ $Taint.Value}}:{{ $Taint.Effect }}"
-{{ end }}
-`
 )
 
 type LaunchTemplateProvider struct {
@@ -103,10 +90,16 @@ func (p *LaunchTemplateProvider) Get(ctx context.Context, provisioner *v1alpha3.
 		return nil, err
 	}
 
+	// 3. Get userData for Node
+	userData, err := p.getUserData(ctx, provisioner, constraints)
+	if err != nil {
+		return nil, err
+	}
+
 	// 4. Ensure the launch template exists, or create it
 	launchTemplate, err := p.ensureLaunchTemplate(ctx, &launchTemplateOptions{
 		Cluster:        provisioner.Spec.Cluster,
-		UserData:       p.getUserData(provisioner, constraints),
+		UserData:       userData,
 		AMIID:          amiID,
 		SecurityGroups: securityGroups,
 	})
@@ -197,14 +190,28 @@ func (p *LaunchTemplateProvider) getSecurityGroupIds(ctx context.Context, provis
 	return securityGroupIds, nil
 }
 
-func (p *LaunchTemplateProvider) getUserData(provisioner *v1alpha3.Provisioner, constraints *Constraints) string {
-	t := template.Must(template.New("userData").Parse(bottlerocketUserData))
+func (p *LaunchTemplateProvider) getUserData(ctx context.Context, provisioner *v1alpha3.Provisioner, constraints *Constraints) (string, error) {
 	var userData bytes.Buffer
-	if err := t.Execute(&userData, struct {
-		Constraints *Constraints
-		Cluster     v1alpha3.Cluster
-	}{constraints, provisioner.Spec.Cluster}); err != nil {
-		panic(fmt.Sprintf("Parsing user data from %v, %v, %s", provisioner, constraints, err.Error()))
+	userData.WriteString(fmt.Sprintf("[settings.kubernetes]\napi-server = \"%s\"\n", provisioner.Spec.Cluster.Endpoint))
+	userData.WriteString(fmt.Sprintf("cluster-name = \"%s\"\n", *provisioner.Spec.Cluster.Name))
+	caBundle, err := provisioner.Spec.Cluster.GetCABundle(ctx)
+	if err != nil {
+		return "", fmt.Errorf("getting user data, %w", err)
+	}
+	if caBundle != nil {
+		userData.WriteString(fmt.Sprintf("cluster-certificate = \"%s\"\n", *caBundle))
+	}
+	if len(constraints.Labels) > 0 {
+		userData.WriteString("[settings.kubernetes.node-labels]\n")
+		for k, v := range constraints.Labels {
+			userData.WriteString(fmt.Sprintf("\"%s\" = \"%v\"\n", k, v))
+		}
+	}
+	if len(constraints.Taints) > 0 {
+		userData.WriteString("[settings.kubernetes.node-taints]\n")
+		for _, taint := range constraints.Taints {
+			userData.WriteString(fmt.Sprintf("\"%s\" = \"%s:%s\"\n", taint.Key, taint.Value, taint.Effect))
+		}
 	}
-	return base64.StdEncoding.EncodeToString(userData.Bytes())
+	return base64.StdEncoding.EncodeToString(userData.Bytes()), nil
 }

pkg/cloudprovider/aws/suite_test.go

@@ -335,7 +335,7 @@ var _ = Describe("Allocation", func() {
 				launchTemplate := input.LaunchTemplateConfigs[0].LaunchTemplateSpecification
 				Expect(*launchTemplate.LaunchTemplateName).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateNameLabel]))
 			})
-			It("should allow a pod to override the launch template id and use the default launch template version", func() {
+			It("should allow a pod to override the launch template name and use the default launch template version", func() {
 				// Setup
 				provisioner.Spec.Labels = map[string]string{LaunchTemplateNameLabel: randomdata.SillyName()}
 				ExpectCreated(env.Client, provisioner)
@@ -352,7 +352,7 @@ var _ = Describe("Allocation", func() {
 				Expect(*launchTemplate.LaunchTemplateName).To(Equal(pods[0].Spec.NodeSelector[LaunchTemplateNameLabel]))
 				Expect(*launchTemplate.Version).To(Equal(DefaultLaunchTemplateVersion))
 			})
-			It("should allow a pod to override the launch template id and use the provisioner's launch template version", func() {
+			It("should allow a pod to override the launch template name and use the provisioner's launch template version", func() {
 				// Setup
 				provisioner.Spec.Labels = map[string]string{
 					LaunchTemplateNameLabel: randomdata.SillyName(),
@@ -372,7 +372,7 @@ var _ = Describe("Allocation", func() {
 			})
 		})
 		Context("Subnets", func() {
-			It("should default to the clusters subnets", func() {
+			It("should default to the cluster's subnets", func() {
 				// Setup
 				provisioner.Spec.InstanceTypes = []string{"m5.large"} // limit instance type to simplify ConsistOf checks
 				ExpectCreated(env.Client, provisioner)

pkg/controllers/allocation/controller.go

@@ -172,15 +172,7 @@ func (c *Controller) provisionerFor(ctx context.Context, name types.NamespacedNa
 	if err := c.KubeClient.Get(ctx, name, provisioner); err != nil {
 		return nil, err
 	}
-
-	// Hydrate provisioner with (dynamic) default values, which must not
-	//    be persisted into the original CRD as they might change with each reconciliation
-	//    loop iteration.
-	defaulted, err := provisioner.WithDynamicDefaults(ctx)
-	if err != nil {
-		return &defaulted, fmt.Errorf("setting dynamic default values, %w", err)
-	}
-	return &defaulted, nil
+	return provisioner, nil
 }
 
 // podToProvisioner is a function handler to transform pod objs to provisioner reconcile requests

pkg/controllers/allocation/scheduling/scheduler.go

@@ -134,7 +134,6 @@ func (s *Scheduler) getDaemons(ctx context.Context, node *v1.Node) ([]*v1.Pod, e
 	return pods, nil
 }
 
-
 // IsSchedulable returns true if the pod can schedule to the node
 func IsSchedulable(pod *v1.Pod, node *v1.Node) bool {
 	// Tolerate Taints

pkg/utils/restconfig/restconfig.go

@@ -0,0 +1,21 @@
+package restconfig
+
+import (
+	"context"
+
+	"k8s.io/client-go/rest"
+)
+
+type contextKey struct{}
+
+func Inject(ctx context.Context, config *rest.Config) context.Context {
+	return context.WithValue(ctx, contextKey{}, config)
+}
+
+func Get(ctx context.Context) *rest.Config {
+	retval := ctx.Value(contextKey{})
+	if retval == nil {
+		return nil
+	}
+	return retval.(*rest.Config)
+}

website/content/en/docs/provisioner-crd.md

@@ -33,10 +33,10 @@ spec:
   # Provisioned nodes will have these labels
   labels:
     ##### AWS Specific #####
-    # Constrain node launch template, default="bottlerocket"
-    node.k8s.aws/launch-template-id: "bottlerocket-qwertyuiop"
-    # Constrain node launch template, default="$LATEST"
-    node.k8s.aws/launch-template-version: "my-special-version"
+    # Constrain node launch template ('$Default' version always used).
+    # If not specified, Karpenter will generate a Bottlerocket-
+    # based launch template dynamically.
+    node.k8s.aws/launch-template-name: "my-launch-template-name"
     # Constrain node capacity type, default="on-demand"
     node.k8s.aws/capacity-type: "spot"
 ```