From 5459b7d37c13c6678af07a1623d6d4b35fe056b6 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Fri, 18 Oct 2019 09:18:20 +0200 Subject: [PATCH] Handle unknown types as unstructured (#954) **What this PR does / why we need it**: When scheme did not contain that type, we previously failed. Since we should be able to handle all types that people might have installed in their cluster, we should fallback to parsing a type as unstructured when not available. This PR has 3 parts: - update to `ParseKubernetesObjects` to process unknown types as Unstructured - use of dynamic restmapper in execution that ensures that we discover new types that were added during the lifetime of the controller - integration test NOTE: Right now we are using dynamic mapper from test harness. That is probably not as robust as the one introduced in the controller-runtime 0.3.0. I will port this code to the controller-runtime mapper when we bump the version of controller-runtime (that should happen prior to 0.8.0 anyway). Fixes #913 --- cmd/manager/main.go | 5 +- go.mod | 3 +- go.sum | 1 + pkg/test/harness.go | 3 +- pkg/test/utils/dynamicrestmapper.go | 3 + pkg/util/template/template.go | 21 +- pkg/util/template/template_test.go | 24 ++ .../operator-with-custom-crd/00-assert.yaml | 4 + .../00-install-crd.yaml | 342 ++++++++++++++++++ .../operator-with-custom-crd/01-assert.yaml | 10 + .../operator-with-custom-crd/01-install.yaml | 4 + .../operator/operator.yaml | 21 ++ .../operator/params.yaml | 0 .../operator/templates/sm.yaml | 14 + 14 files changed, 448 insertions(+), 7 deletions(-) create mode 100644 pkg/util/template/template_test.go create mode 100644 test/integration/operator-with-custom-crd/00-assert.yaml create mode 100644 test/integration/operator-with-custom-crd/00-install-crd.yaml create mode 100644 test/integration/operator-with-custom-crd/01-assert.yaml create mode 100644 test/integration/operator-with-custom-crd/01-install.yaml create mode 100644 test/integration/operator-with-custom-crd/operator/operator.yaml create mode 100644 test/integration/operator-with-custom-crd/operator/params.yaml create mode 100644 test/integration/operator-with-custom-crd/operator/templates/sm.yaml diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 3cf31e7c8..b23a6c4fa 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -23,6 +23,7 @@ import ( "github.com/kudobuilder/kudo/pkg/controller/instance" "github.com/kudobuilder/kudo/pkg/controller/operator" "github.com/kudobuilder/kudo/pkg/controller/operatorversion" + util "github.com/kudobuilder/kudo/pkg/test/utils" "github.com/kudobuilder/kudo/pkg/version" apiextenstionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -40,7 +41,9 @@ func main() { // create new controller-runtime manager log.Info("setting up manager") - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{}) + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + MapperProvider: util.NewDynamicRESTMapper, + }) if err != nil { log.Error(err, "unable to start manager") os.Exit(1) diff --git a/go.mod b/go.mod index a3c47fae9..287ec485b 100644 --- a/go.mod +++ b/go.mod @@ -52,8 +52,9 @@ require ( golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a // indirect golang.org/x/sys v0.0.0-20190911201528-7ad0cfa0b7b5 // indirect - golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect + golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 golang.org/x/tools v0.0.0-20190909030654-5b82db07426d + golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 google.golang.org/appengine v1.5.0 // indirect google.golang.org/grpc v1.21.0 // indirect gopkg.in/yaml.v2 v2.2.2 diff --git a/go.sum b/go.sum index 5bac424d9..e9c165b4f 100644 --- a/go.sum +++ b/go.sum @@ -433,6 +433,7 @@ golang.org/x/tools v0.0.0-20190501045030-23463209683d/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190909030654-5b82db07426d h1:PhtdWYteEBebOX7KXm4qkIAVSUTHQ883/2hRB92r9lk= golang.org/x/tools v0.0.0-20190909030654-5b82db07426d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0= gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= diff --git a/pkg/test/harness.go b/pkg/test/harness.go index 58c327948..ec08c7fbd 100644 --- a/pkg/test/harness.go +++ b/pkg/test/harness.go @@ -250,7 +250,8 @@ func (h *Harness) RunKUDO() error { } mgr, err := manager.New(config, manager.Options{ - Scheme: testutils.Scheme(), + Scheme: testutils.Scheme(), + MapperProvider: testutils.NewDynamicRESTMapper, }) if err != nil { return err diff --git a/pkg/test/utils/dynamicrestmapper.go b/pkg/test/utils/dynamicrestmapper.go index a1ce92f1b..1514286b6 100644 --- a/pkg/test/utils/dynamicrestmapper.go +++ b/pkg/test/utils/dynamicrestmapper.go @@ -5,6 +5,8 @@ package utils // This helps work around issues with the DiscoveryRESTMapper caching resources. import ( + "fmt" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -52,6 +54,7 @@ func (drm *DynamicRESTMapper) reloadOnError(err error) bool { return false } err = drm.reload() + fmt.Println("reload") if err != nil { utilruntime.HandleError(err) } diff --git a/pkg/util/template/template.go b/pkg/util/template/template.go index 91a067dcc..99277b1e5 100644 --- a/pkg/util/template/template.go +++ b/pkg/util/template/template.go @@ -1,14 +1,20 @@ package template import ( + "bytes" "strings" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + yamlutil "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/client-go/kubernetes/scheme" ) -//ParseKubernetesObjects parses a list of runtime.Objects from the provided yaml +// ParseKubernetesObjects parses a list of runtime.Objects from the provided yaml +// If the type is not known in the scheme, it tries to parse it as Unstructured +// TODO(av) could we use something else than a global scheme here? Should we somehow inject it? func ParseKubernetesObjects(yaml string) (objs []runtime.Object, err error) { sepYamlfiles := strings.Split(yaml, "---") for _, f := range sepYamlfiles { @@ -21,10 +27,17 @@ func ParseKubernetesObjects(yaml string) (objs []runtime.Object, err error) { obj, _, e := decode([]byte(f), nil, nil) if e != nil { - err = e - return + // if parsing to scheme known types fails, just try to parse into unstructured + unstructuredObj := &unstructured.Unstructured{} + fileBytes := []byte(f) + decoder := yamlutil.NewYAMLOrJSONDecoder(bytes.NewBuffer(fileBytes), len(fileBytes)) + if err = decoder.Decode(unstructuredObj); err != nil { + return nil, err + } + objs = append(objs, unstructuredObj) + } else { + objs = append(objs, obj) } - objs = append(objs, obj) } return } diff --git a/pkg/util/template/template_test.go b/pkg/util/template/template_test.go new file mode 100644 index 000000000..99060eb2c --- /dev/null +++ b/pkg/util/template/template_test.go @@ -0,0 +1,24 @@ +package template + +import "testing" + +func TestParseKubernetesObjects_UnknownType(t *testing.T) { + _, err := ParseKubernetesObjects(`apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + labels: + app: prometheus-operator + release: prometheus-kubeaddons + name: spark-cluster-monitor +spec: + endpoints: + - interval: 5s + port: metrics + selector: + matchLabels: + spark/servicemonitor: true`) + + if err != nil { + t.Errorf("Expecting no error but got %s", err) + } +} diff --git a/test/integration/operator-with-custom-crd/00-assert.yaml b/test/integration/operator-with-custom-crd/00-assert.yaml new file mode 100644 index 000000000..42d7da9a5 --- /dev/null +++ b/test/integration/operator-with-custom-crd/00-assert.yaml @@ -0,0 +1,4 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: servicemonitors.monitoring.coreos.com \ No newline at end of file diff --git a/test/integration/operator-with-custom-crd/00-install-crd.yaml b/test/integration/operator-with-custom-crd/00-install-crd.yaml new file mode 100644 index 000000000..48b4ab9d9 --- /dev/null +++ b/test/integration/operator-with-custom-crd/00-install-crd.yaml @@ -0,0 +1,342 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + creationTimestamp: null + name: servicemonitors.monitoring.coreos.com +spec: + group: monitoring.coreos.com + names: + kind: ServiceMonitor + plural: servicemonitors + scope: Namespaced + validation: + openAPIV3Schema: + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + spec: + description: ServiceMonitorSpec contains specification parameters for a + ServiceMonitor. + properties: + endpoints: + description: A list of endpoints allowed as part of this ServiceMonitor. + items: + description: Endpoint defines a scrapeable endpoint serving Prometheus + metrics. + properties: + basicAuth: + description: 'BasicAuth allow an endpoint to authenticate over + basic authentication More info: https://prometheus.io/docs/operating/configuration/#endpoints' + properties: + password: + description: SecretKeySelector selects a key of a Secret. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + username: + description: SecretKeySelector selects a key of a Secret. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + type: object + bearerTokenFile: + description: File to read bearer token for scraping targets. + type: string + bearerTokenSecret: + description: SecretKeySelector selects a key of a Secret. + properties: + key: + description: The key of the secret to select from. Must be + a valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + optional: + description: Specify whether the Secret or its key must be + defined + type: boolean + required: + - key + type: object + honorLabels: + description: HonorLabels chooses the metric's labels on collisions + with target labels. + type: boolean + interval: + description: Interval at which metrics should be scraped + type: string + metricRelabelings: + description: MetricRelabelConfigs to apply to samples before ingestion. + items: + description: 'RelabelConfig allows dynamic rewriting of the + label set, being applied to samples before ingestion. It defines + ``-section of Prometheus configuration. + More info: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#metric_relabel_configs' + properties: + action: + description: Action to perform based on regex matching. + Default is 'replace' + type: string + modulus: + description: Modulus to take of the hash of the source label + values. + format: int64 + type: integer + regex: + description: Regular expression against which the extracted + value is matched. defailt is '(.*)' + type: string + replacement: + description: Replacement value against which a regex replace + is performed if the regular expression matches. Regex + capture groups are available. Default is '$1' + type: string + separator: + description: Separator placed between concatenated source + label values. default is ';'. + type: string + sourceLabels: + description: The source labels select values from existing + labels. Their content is concatenated using the configured + separator and matched against the configured regular expression + for the replace, keep, and drop actions. + items: + type: string + type: array + targetLabel: + description: Label to which the resulting value is written + in a replace action. It is mandatory for replace actions. + Regex capture groups are available. + type: string + type: object + type: array + params: + description: Optional HTTP URL parameters + type: object + path: + description: HTTP path to scrape for metrics. + type: string + port: + description: Name of the service port this endpoint refers to. + Mutually exclusive with targetPort. + type: string + proxyUrl: + description: ProxyURL eg http://proxyserver:2195 Directs scrapes + to proxy through this endpoint. + type: string + relabelings: + description: 'RelabelConfigs to apply to samples before scraping. + More info: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config' + items: + description: 'RelabelConfig allows dynamic rewriting of the + label set, being applied to samples before ingestion. It defines + ``-section of Prometheus configuration. + More info: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#metric_relabel_configs' + properties: + action: + description: Action to perform based on regex matching. + Default is 'replace' + type: string + modulus: + description: Modulus to take of the hash of the source label + values. + format: int64 + type: integer + regex: + description: Regular expression against which the extracted + value is matched. defailt is '(.*)' + type: string + replacement: + description: Replacement value against which a regex replace + is performed if the regular expression matches. Regex + capture groups are available. Default is '$1' + type: string + separator: + description: Separator placed between concatenated source + label values. default is ';'. + type: string + sourceLabels: + description: The source labels select values from existing + labels. Their content is concatenated using the configured + separator and matched against the configured regular expression + for the replace, keep, and drop actions. + items: + type: string + type: array + targetLabel: + description: Label to which the resulting value is written + in a replace action. It is mandatory for replace actions. + Regex capture groups are available. + type: string + type: object + type: array + scheme: + description: HTTP scheme to use for scraping. + type: string + scrapeTimeout: + description: Timeout after which the scrape is ended + type: string + targetPort: + anyOf: + - type: string + - type: integer + tlsConfig: + description: TLSConfig specifies TLS configuration parameters. + properties: + ca: {} + caFile: + description: Path to the CA cert in the Prometheus container + to use for the targets. + type: string + cert: {} + certFile: + description: Path to the client cert file in the Prometheus + container for the targets. + type: string + insecureSkipVerify: + description: Disable target certificate validation. + type: boolean + keyFile: + description: Path to the client key file in the Prometheus + container for the targets. + type: string + keySecret: + description: SecretKeySelector selects a key of a Secret. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + optional: + description: Specify whether the Secret or its key must + be defined + type: boolean + required: + - key + type: object + serverName: + description: Used to verify the hostname for the targets. + type: string + type: object + type: object + type: array + jobLabel: + description: The label to use to retrieve the job name from. + type: string + namespaceSelector: + description: NamespaceSelector is a selector for selecting either all + namespaces or a list of namespaces. + properties: + any: + description: Boolean describing whether all namespaces are selected + in contrast to a list restricting them. + type: boolean + matchNames: + description: List of namespace names. + items: + type: string + type: array + type: object + podTargetLabels: + description: PodTargetLabels transfers labels on the Kubernetes Pod + onto the target. + items: + type: string + type: array + sampleLimit: + description: SampleLimit defines per-scrape limit on number of scraped + samples that will be accepted. + format: int64 + type: integer + selector: + description: A label selector is a label query over a set of resources. + The result of matchLabels and matchExpressions are ANDed. An empty + label selector matches all objects. A null label selector matches + no objects. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains + values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: operator represents a key's relationship to a + set of values. Valid operators are In, NotIn, Exists and + DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator + is In or NotIn, the values array must be non-empty. If the + operator is Exists or DoesNotExist, the values array must + be empty. This array is replaced during a strategic merge + patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + description: matchLabels is a map of {key,value} pairs. A single + {key,value} in the matchLabels map is equivalent to an element + of matchExpressions, whose key field is "key", the operator is + "In", and the values array contains only "value". The requirements + are ANDed. + type: object + type: object + targetLabels: + description: TargetLabels transfers labels on the Kubernetes Service + onto the target. + items: + type: string + type: array + required: + - endpoints + - selector + type: object + type: object + version: v1 \ No newline at end of file diff --git a/test/integration/operator-with-custom-crd/01-assert.yaml b/test/integration/operator-with-custom-crd/01-assert.yaml new file mode 100644 index 000000000..ddf43285e --- /dev/null +++ b/test/integration/operator-with-custom-crd/01-assert.yaml @@ -0,0 +1,10 @@ +apiVersion: kudo.dev/v1alpha1 +kind: Instance +metadata: + name: crd-instance +spec: + operatorVersion: + name: crd-operator-0.1.0 +status: + aggregatedStatus: + status: COMPLETE \ No newline at end of file diff --git a/test/integration/operator-with-custom-crd/01-install.yaml b/test/integration/operator-with-custom-crd/01-install.yaml new file mode 100644 index 000000000..3d0a79636 --- /dev/null +++ b/test/integration/operator-with-custom-crd/01-install.yaml @@ -0,0 +1,4 @@ +apiVersion: kudo.dev/v1alpha1 +kind: TestStep +kubectl: + - kudo install --instance crd-instance ./operator diff --git a/test/integration/operator-with-custom-crd/operator/operator.yaml b/test/integration/operator-with-custom-crd/operator/operator.yaml new file mode 100644 index 000000000..58e1dd505 --- /dev/null +++ b/test/integration/operator-with-custom-crd/operator/operator.yaml @@ -0,0 +1,21 @@ +name: "crd-operator" +version: "0.1.0" +kubernetesVersion: 1.13 +maintainers: + - name: Your name + email: +url: https://kudo.dev +tasks: + crd: + resources: + - sm.yaml +plans: + deploy: + strategy: serial + phases: + - name: main + strategy: parallel + steps: + - name: everything + tasks: + - crd diff --git a/test/integration/operator-with-custom-crd/operator/params.yaml b/test/integration/operator-with-custom-crd/operator/params.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/test/integration/operator-with-custom-crd/operator/templates/sm.yaml b/test/integration/operator-with-custom-crd/operator/templates/sm.yaml new file mode 100644 index 000000000..426a941f9 --- /dev/null +++ b/test/integration/operator-with-custom-crd/operator/templates/sm.yaml @@ -0,0 +1,14 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + labels: + app: prometheus-operator + release: prometheus-kubeaddons + name: spark-cluster-monitor +spec: + endpoints: + - interval: 5s + port: metrics + selector: + matchLabels: + spark/servicemonitor: true \ No newline at end of file