Fix first round of issues on proposal

Signed-off-by: killianmuldoon <[email protected]>

.golangci.yml

@@ -108,6 +108,9 @@ linters-settings:
         alias: addonsv1alpha4
       - pkg: sigs.k8s.io/cluster-api/exp/addons/api/v1beta1
         alias: addonsv1
+      # CAPI exp runtime
+      - pkg: sigs.k8s.io/cluster-api/exp/runtime/api/v1beta1
+        alias: runtimev1
       # CAPD
       - pkg: sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1alpha3
         alias: infrav1alpha3

config/crd/bases/runtime.cluster.x-k8s.io_extensionconfigs.yaml

@@ -27,7 +27,7 @@ spec:
     name: v1beta1
     schema:
       openAPIV3Schema:
-        description: ExtensionConfig is the Schema for the ExtensionHandlers API.
+        description: ExtensionConfig is the Schema for the ExtensionConfig API.
         properties:
           apiVersion:
             description: 'APIVersion defines the versioned schema of this representation
@@ -45,7 +45,7 @@ spec:
             description: ExtensionConfigSpec is the desired state of the ExtensionConfig
             properties:
               clientConfig:
-                description: ClientConfig defines how to communicate with the ExtensionHandler.
+                description: ClientConfig defines how to communicate with ExtensionHandlers.
                 properties:
                   caBundle:
                     description: CABundle is a PEM encoded CA bundle which will be
@@ -71,8 +71,8 @@ spec:
                         type: string
                       port:
                         description: Port is the port on the service that hosting
-                          ExtensionHandler. Default to 443 for backward compatibility.
-                          `port` should be a valid port number (1-65535, inclusive).
+                          ExtensionHandler. Default to 8443. `port` should be a valid
+                          port number (1-65535, inclusive).
                         format: int32
                         type: integer
                     required:
@@ -84,23 +84,21 @@ spec:
                       in standard URL form (`scheme://host:port/path`). Exactly one
                       of `url` or `service` must be specified. \n The `host` should
                       not refer to a service running in the cluster; use the `service`
-                      field instead. \n Please note that using `localhost` or `127.0.0.1`
-                      as a `host` is risky unless you take great care to run the extension
-                      this ExtensionConfig references on all hosts which run a controller
-                      which might need to make calls to this ExtensionHandler. Such
-                      installs are likely to be non-portable, i.e., not easy to turn
-                      up in a new cluster. \n The scheme should be \"https\"; the
-                      URL should begin with \"https://\". \"http\" is supported for
-                      insecure development purposes only. \n A path is optional, and
-                      if present may be any string permissible in a URL. If a path
-                      is set it will be used as prefix and the hook-specific path
-                      will be appended. \n Attempting to use a user or basic auth
-                      e.g. \"user:password@\" is not allowed. Fragments (\"#...\")
-                      and query parameters (\"?...\") are not allowed either."
+                      field instead. \n The scheme should be \"https\"; the URL should
+                      begin with \"https://\". \"http\" is supported for insecure
+                      development purposes only. \n A path is optional, and if present
+                      may be any string permissible in a URL. If a path is set it
+                      will be used as prefix and the hook-specific path will be appended.
+                      \n Attempting to use a user or basic auth e.g. \"user:password@\"
+                      is not allowed. Fragments (\"#...\") and query parameters (\"?...\")
+                      are not allowed either."
                     type: string
                 type: object
               namespaceSelector:
-                description: Default to the empty LabelSelector, which matches everything.
+                description: NamespaceSelector decides whether to run the webhook
+                  on an object based on whether the namespace for that object matches
+                  the selector. Default to the empty LabelSelector, which matches
+                  everything.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.
@@ -149,44 +147,6 @@ spec:
           status:
             description: ExtensionConfigStatus is the current state of the ExtensionConfig
             properties:
-              ExtensionHandlers:
-                description: ExtensionHandlers defines the current ExtensionHandlers
-                  supported by an Extension.
-                items:
-                  description: ExtensionHandler specifies the details of a handler
-                    for a particular runtime hook registered by an Extension server.
-                  properties:
-                    failurePolicy:
-                      description: FailurePolicy defines how failures in calls to
-                        the ExtensionHandler should be handled by a client.
-                      type: string
-                    hook:
-                      description: Hook defines the versioned runtime event which
-                        this ExtensionHandler serves.
-                      properties:
-                        apiVersion:
-                          description: APIVersion is the Version of the Hook
-                          type: string
-                        hook:
-                          description: Hook is the name of the hook
-                          type: string
-                      required:
-                      - apiVersion
-                      - hook
-                      type: object
-                    name:
-                      description: Name is the name of the ExtensionHandler.
-                      type: string
-                    timeoutSeconds:
-                      description: TimeoutSeconds defines the timeout duration for
-                        client calls to the ExtensionHandler.
-                      format: int32
-                      type: integer
-                  required:
-                  - hook
-                  - name
-                  type: object
-                type: array
               conditions:
                 description: Conditions define the current service state of the ExtensionConfig.
                 items:
@@ -232,6 +192,44 @@ spec:
                   - type
                   type: object
                 type: array
+              extensionHandlers:
+                description: ExtensionHandlers defines the current ExtensionHandlers
+                  supported by an Extension.
+                items:
+                  description: ExtensionHandler specifies the details of a handler
+                    for a particular runtime hook registered by an Extension server.
+                  properties:
+                    failurePolicy:
+                      description: FailurePolicy defines how failures in calls to
+                        the ExtensionHandler should be handled by a client.
+                      type: string
+                    hook:
+                      description: Hook defines the versioned runtime hook which this
+                        ExtensionHandler serves.
+                      properties:
+                        apiVersion:
+                          description: APIVersion is the Version of the Hook.
+                          type: string
+                        hook:
+                          description: Hook is the name of the hook.
+                          type: string
+                      required:
+                      - apiVersion
+                      - hook
+                      type: object
+                    name:
+                      description: Name is the unique name of the ExtensionHandler.
+                      type: string
+                    timeoutSeconds:
+                      description: TimeoutSeconds defines the timeout duration for
+                        client calls to the ExtensionHandler.
+                      format: int32
+                      type: integer
+                  required:
+                  - hook
+                  - name
+                  type: object
+                type: array
             type: object
         type: object
     served: true

config/webhook/manifests.yaml

@@ -201,7 +201,7 @@ webhooks:
     - CREATE
     - UPDATE
     resources:
-    - extensionconfiss
+    - extensionconfigs
   sideEffects: None
 ---
 apiVersion: admissionregistration.k8s.io/v1

exp/runtime/api/v1beta1/extension_types.go → exp/runtime/api/v1beta1/extensionconfig_types.go

@@ -26,16 +26,18 @@ import (
 
 // ExtensionConfigSpec defines the desired state of ExtensionConfig.
 type ExtensionConfigSpec struct {
-	// ClientConfig defines how to communicate with the ExtensionHandler.
+	// ClientConfig defines how to communicate with ExtensionHandlers.
 	ClientConfig ClientConfig `json:"clientConfig"`
 
+	// NamespaceSelector decides whether to run the webhook on an object based
+	// on whether the namespace for that object matches the selector.
 	// Default to the empty LabelSelector, which matches everything.
 	// +optional
 	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 }
 
 // ClientConfig contains the information to make a client
-// connection with the ExtensionHandler.
+// connection with an ExtensionHandler.
 type ClientConfig struct {
 	// URL gives the location of the ExtensionHandler, in standard URL form
 	// (`scheme://host:port/path`). Exactly one of `url` or `service`
@@ -44,13 +46,7 @@ type ClientConfig struct {
 	// The `host` should not refer to a service running in the cluster; use
 	// the `service` field instead.
 	//
-	// Please note that using `localhost` or `127.0.0.1` as a `host` is
-	// risky unless you take great care to run the extension this ExtensionConfig references
-	// on all hosts which run a controller which might need to make calls to this
-	// ExtensionHandler. Such installs are likely to be non-portable, i.e., not easy
-	// to turn up in a new cluster.
-	//
-	// The scheme should be "https"; the URL should begin with "https://".
+	//The scheme should be "https"; the URL should begin with "https://".
 	// "http" is supported for insecure development purposes only.
 	//
 	// A path is optional, and if present may be any string permissible in
@@ -92,7 +88,7 @@ type ServiceReference struct {
 	Path *string `json:"path,omitempty"`
 
 	// Port is the port on the service that hosting ExtensionHandler.
-	// Default to 443 for backward compatibility.
+	// Default to 8443.
 	// `port` should be a valid port number (1-65535, inclusive).
 	// +optional
 	Port *int32 `json:"port,omitempty"`
@@ -105,7 +101,7 @@ type ServiceReference struct {
 // ExtensionConfigStatus defines the observed state of ExtensionConfig.
 type ExtensionConfigStatus struct {
 	// ExtensionHandlers defines the current ExtensionHandlers supported by an Extension.
-	ExtensionHandlers []ExtensionHandler `json:"ExtensionHandlers,omitempty"`
+	ExtensionHandlers []ExtensionHandler `json:"extensionHandlers,omitempty"`
 
 	// Conditions define the current service state of the ExtensionConfig.
 	// +optional
@@ -114,10 +110,10 @@ type ExtensionConfigStatus struct {
 
 // ExtensionHandler specifies the details of a handler for a particular runtime hook registered by an Extension server.
 type ExtensionHandler struct {
-	// Name is the name of the ExtensionHandler.
+	// Name is the unique name of the ExtensionHandler.
 	Name string `json:"name"`
 
-	// Hook defines the versioned runtime event which this ExtensionHandler serves.
+	// Hook defines the versioned runtime hook which this ExtensionHandler serves.
 	Hook GroupVersionHook `json:"hook"`
 
 	// TimeoutSeconds defines the timeout duration for client calls to the ExtensionHandler.
@@ -127,12 +123,12 @@ type ExtensionHandler struct {
 	FailurePolicy *FailurePolicy `json:"failurePolicy,omitempty"`
 }
 
-// GroupVersionHook defines the runtime event when the ExtensionHandler is called.
+// GroupVersionHook defines the runtime hook when the ExtensionHandler is called.
 type GroupVersionHook struct {
-	// APIVersion is the Version of the Hook
+	// APIVersion is the Version of the Hook.
 	APIVersion string `json:"apiVersion"`
 
-	// Hook is the name of the hook
+	// Hook is the name of the hook.
 	Hook string `json:"hook"`
 }
 
@@ -143,7 +139,7 @@ const (
 	// FailurePolicyIgnore means that an error calling the extension is ignored.
 	FailurePolicyIgnore FailurePolicy = "Ignore"
 
-	// FailurePolicyFail means that an error calling the extension causes the admission to fail.
+	// FailurePolicyFail means that an error calling the extension is propagated as an error.
 	FailurePolicyFail FailurePolicy = "Fail"
 )
 
@@ -154,9 +150,8 @@ const (
 // +kubebuilder:subresource:status
 // +kubebuilder:storageversion
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Time duration since creation of ExtensionConfig"
-// +k8s:conversion-gen=false
 
-// ExtensionConfig is the Schema for the ExtensionHandlers API.
+// ExtensionConfig is the Schema for the ExtensionConfig API.
 type ExtensionConfig struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

exp/runtime/api/v1beta1/extension_webhook.go → exp/runtime/api/v1beta1/extensionconfig_webhook.go

@@ -36,7 +36,7 @@ func (e *ExtensionConfig) SetupWebhookWithManager(mgr ctrl.Manager) error {
 }
 
 // +kubebuilder:webhook:verbs=create;update,path=/validate-runtime-cluster-x-k8s-io-v1beta1-extensionconfig,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=runtime.cluster.x-k8s.io,resources=extensionconfigs,versions=v1beta1,name=validation.extensionconfig.runtime.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
-// +kubebuilder:webhook:verbs=create;update,path=/mutate-runtime-cluster-x-k8s-io-v1beta1-extensionconfig,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=runtime.cluster.x-k8s.io,resources=extensionconfiss,versions=v1beta1,name=default.extensionconfig.runtime.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
+// +kubebuilder:webhook:verbs=create;update,path=/mutate-runtime-cluster-x-k8s-io-v1beta1-extensionconfig,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=runtime.cluster.x-k8s.io,resources=extensionconfigs,versions=v1beta1,name=default.extensionconfig.runtime.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
 var _ webhook.Validator = &ExtensionConfig{}
 var _ webhook.Defaulter = &ExtensionConfig{}
@@ -66,7 +66,7 @@ func (e *ExtensionConfig) ValidateUpdate(old runtime.Object) error {
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
 func (e *ExtensionConfig) validate(_ *ExtensionConfig) error {
 	// NOTE: ExtensionConfig is behind the RuntimeSDK feature gate flag; the web hook
-	// must prevent updating objects in case the feature flag is disabled.
+	// must prevent createing and updating objects in case the feature flag is disabled.
 	if !feature.Gates.Enabled(feature.RuntimeSDK) {
 		return field.Forbidden(
 			field.NewPath("spec"),

feature/feature.go

@@ -47,7 +47,7 @@ const (
 
 	// RuntimeSDK is a feature gate for the Runtime hooks and extensions functionality.
 	//
-	// alpha: v1.1
+	// alpha: v1.2
 	RuntimeSDK featuregate.Feature = "RuntimeSDK"
 
 	// KubeadmBootstrapFormatIgnition is a feature gate for the Ignition bootstrap format