From 36eaeeec4b0fbb0bdf421d0aae81019f68c4840d Mon Sep 17 00:00:00 2001 From: Kuat Date: Fri, 26 Apr 2019 10:00:30 -0700 Subject: [PATCH] Re-enable Mixer validation (#13379) * cleaning up mixer validation Signed-off-by: Kuat Yessenov * fixes Signed-off-by: Kuat Yessenov * fix mixer tests Signed-off-by: Kuat Yessenov * fix galley test Signed-off-by: Kuat Yessenov * less diff Signed-off-by: Kuat Yessenov * no edge case possible Signed-off-by: Kuat Yessenov * fixing the adapter dependencies Signed-off-by: Kuat Yessenov * enable validation Signed-off-by: Kuat Yessenov * goimports Signed-off-by: Kuat Yessenov * missed an adapter Signed-off-by: Kuat Yessenov * edge case Signed-off-by: Kuat Yessenov * coverage Signed-off-by: Kuat Yessenov --- galley/pkg/crd/validation/validation.go | 24 +- galley/pkg/crd/validation/webhook.go | 9 +- galley/pkg/crd/validation/webhook_test.go | 6 +- istioctl/pkg/validate/validate.go | 70 ++-- istioctl/pkg/validate/validate_test.go | 11 +- mixer/adapter/bypass/bypass.go | 17 +- mixer/adapter/circonus/circonus.go | 13 +- mixer/adapter/cloudwatch/cloudwatch.go | 14 +- mixer/adapter/denier/denier.go | 16 +- mixer/adapter/dogstatsd/dogstatsd.go | 18 +- mixer/adapter/fluentd/fluentd.go | 19 +- mixer/adapter/kubernetesenv/kubernetesenv.go | 15 +- mixer/adapter/list/list.go | 20 +- mixer/adapter/memquota/memquota.go | 17 +- mixer/adapter/metadata/inventory.go | 360 ++++++++++++++++++ mixer/adapter/noop/noop.go | 23 +- mixer/adapter/opa/opa.go | 14 +- mixer/adapter/prometheus/prometheus.go | 14 +- mixer/adapter/rbac/rbac.go | 15 +- mixer/adapter/redisquota/redisquota.go | 17 +- mixer/adapter/signalfx/signalfx.go | 20 +- mixer/adapter/solarwinds/solarwinds.go | 15 +- mixer/adapter/stackdriver/stackdriver.go | 31 +- mixer/adapter/statsd/statsd.go | 21 +- mixer/adapter/stdio/stdio.go | 40 +- mixer/adapter/zipkin/zipkin.go | 17 +- mixer/pkg/config/store/convert.go | 45 ++- mixer/pkg/config/store/convert_test.go | 15 + mixer/pkg/config/store/queue.go | 24 +- mixer/pkg/config/store/store.go | 1 + mixer/pkg/config/store/validator.go | 99 ----- mixer/pkg/config/store/validator_test.go | 115 ------ mixer/pkg/runtime/config/ephemeral.go | 3 +- .../pkg/runtime/config/validator/validator.go | 122 ------ .../config/validator/validator_test.go | 344 ----------------- mixer/pkg/runtime/runtime.go | 6 +- mixer/pkg/runtime/testing/data/data.go | 6 +- mixer/pkg/validate/validator.go | 87 +++++ mixer/pkg/validate/validator_test.go | 90 +++++ mixer/test/e2e/e2e_report_test.go | 6 +- 40 files changed, 723 insertions(+), 1096 deletions(-) create mode 100644 mixer/adapter/metadata/inventory.go delete mode 100644 mixer/pkg/config/store/validator.go delete mode 100644 mixer/pkg/config/store/validator_test.go delete mode 100644 mixer/pkg/runtime/config/validator/validator.go delete mode 100644 mixer/pkg/runtime/config/validator/validator_test.go create mode 100644 mixer/pkg/validate/validator.go create mode 100644 mixer/pkg/validate/validator_test.go diff --git a/galley/pkg/crd/validation/validation.go b/galley/pkg/crd/validation/validation.go index 87a15b13a6ab..9fd56eb7ab34 100644 --- a/galley/pkg/crd/validation/validation.go +++ b/galley/pkg/crd/validation/validation.go @@ -25,12 +25,7 @@ import ( multierror "github.com/hashicorp/go-multierror" - "istio.io/istio/mixer/adapter" - "istio.io/istio/mixer/pkg/config" - "istio.io/istio/mixer/pkg/config/store" - runtimeConfig "istio.io/istio/mixer/pkg/runtime/config" - "istio.io/istio/mixer/pkg/template" - generatedTmplRepo "istio.io/istio/mixer/template" + mixervalidate "istio.io/istio/mixer/pkg/validate" "istio.io/istio/pilot/pkg/model" "istio.io/istio/pkg/cmd" "istio.io/istio/pkg/kube" @@ -52,21 +47,6 @@ type httpClient interface { Do(req *http.Request) (*http.Response, error) } -// createMixerValidator creates a mixer backend validator. -// TODO(https://github.com/istio/istio/issues/4887) - refactor mixer -// config validation to remove galley dependency on mixer internal -// packages. -func createMixerValidator() store.BackendValidator { - info := generatedTmplRepo.SupportedTmplInfo - templates := make(map[string]*template.Info, len(info)) - for k := range info { - t := info[k] - templates[k] = &t - } - adapters := config.AdapterInfoMap(adapter.Inventory(), template.NewRepository(info).SupportsTemplate) - return store.NewValidator(nil, runtimeConfig.KindMap(adapters, templates)) -} - func webhookHTTPSHandlerReady(client httpClient, vc *WebhookParameters) error { readinessURL := &url.URL{ Scheme: "https", @@ -95,7 +75,7 @@ func webhookHTTPSHandlerReady(client httpClient, vc *WebhookParameters) error { func RunValidation(vc *WebhookParameters, kubeConfig string, livenessProbeController, readinessProbeController probe.Controller) { log.Infof("Galley validation started with\n%s", vc) - mixerValidator := createMixerValidator() + mixerValidator := mixervalidate.NewDefaultValidator(false) clientset, err := kube.CreateClientset(kubeConfig, "") if err != nil { log.Fatalf("could not create k8s clientset: %v", err) diff --git a/galley/pkg/crd/validation/webhook.go b/galley/pkg/crd/validation/webhook.go index ea16fc6db81c..3b4e0f59f462 100644 --- a/galley/pkg/crd/validation/webhook.go +++ b/galley/pkg/crd/validation/webhook.go @@ -521,9 +521,12 @@ func (wh *Webhook) admitMixer(request *admissionv1beta1.AdmissionRequest) *admis return &admissionv1beta1.AdmissionResponse{Allowed: true} } - if err := wh.validator.Validate(ev); err != nil { - reportValidationFailed(request, reasonInvalidConfig) - return toAdmissionResponse(err) + // webhook skips deletions + if ev.Type == store.Update { + if err := wh.validator.Validate(ev); err != nil { + reportValidationFailed(request, reasonInvalidConfig) + return toAdmissionResponse(err) + } } reportValidationPass(request) diff --git a/galley/pkg/crd/validation/webhook_test.go b/galley/pkg/crd/validation/webhook_test.go index fe85435b88ec..dd969c17add6 100644 --- a/galley/pkg/crd/validation/webhook_test.go +++ b/galley/pkg/crd/validation/webhook_test.go @@ -60,6 +60,10 @@ func (fv *fakeValidator) Validate(*store.BackendEvent) error { return fv.err } +func (fv *fakeValidator) SupportsKind(string) bool { + return true +} + var ( dummyConfig = &admissionregistrationv1beta1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ @@ -432,7 +436,7 @@ func TestAdmitMixer(t *testing.T) { Operation: admissionv1beta1.Delete, }, validator: &fakeValidator{errors.New("fail")}, - allowed: false, + allowed: true, }, { name: "invalid delete (missing name)", diff --git a/istioctl/pkg/validate/validate.go b/istioctl/pkg/validate/validate.go index a995d6ae5dcf..0d6b51f694e0 100644 --- a/istioctl/pkg/validate/validate.go +++ b/istioctl/pkg/validate/validate.go @@ -27,6 +27,9 @@ import ( "k8s.io/kubernetes/pkg/kubectl/genericclioptions" "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" + mixercrd "istio.io/istio/mixer/pkg/config/crd" + mixerstore "istio.io/istio/mixer/pkg/config/store" + mixervalidate "istio.io/istio/mixer/pkg/validate" "istio.io/istio/pilot/pkg/config/kube/crd" "istio.io/istio/pilot/pkg/model" ) @@ -44,45 +47,15 @@ var ( "spec": true, "status": true, } -) - -/* - -TODO(https://github.com/istio/istio/issues/4887) - -Reusing the existing mixer validation code pulls in all of the mixer -adapter packages into istioctl. Not only is this not ideal (see -issue), but it also breaks the istioctl build on windows as some mixer -adapters use linux specific packges (e.g. syslog). - -func createMixerValidator() store.BackendValidator { - info := generatedTmplRepo.SupportedTmplInfo - templates := make(map[string]*template.Info, len(info)) - for k := range info { - t := info[k] - templates[k] = &t - } - adapters := config.AdapterInfoMap(adapter.Inventory(), template.NewRepository(info).SupportsTemplate) - return store.NewValidator(nil, runtimeConfig.KindMap(adapters, templates)) -} -var mixerValidator = createMixerValidator() + mixerAPIVersion = "config.istio.io/v1alpha2" +) -type validateArgs struct { - filenames []string - // TODO validateObjectStream namespace/object? +type validator struct { + mixerValidator mixerstore.BackendValidator } -func (args validateArgs) validate() error { - var errs *multierror.Error - if len(args.filenames) == 0 { - errs = multierror.Append(errs, errors.New("no filenames set (see --filename/-f)")) - } - return errs.ErrorOrNil() -} -*/ - -func validateResource(un *unstructured.Unstructured) error { +func (v *validator) validateResource(un *unstructured.Unstructured) error { schema, exists := model.IstioConfigTypes.GetByType(crd.CamelCaseToKebabCase(un.GetKind())) if exists { obj, err := crd.ConvertObjectFromUnstructured(schema, un, "") @@ -91,20 +64,22 @@ func validateResource(un *unstructured.Unstructured) error { } return schema.Validate(obj.Name, obj.Namespace, obj.Spec) } - return fmt.Errorf("%s.%s validation is not supported", un.GetKind(), un.GetAPIVersion()) - /* - TODO(https://github.com/istio/istio/issues/4887) - ev := &store.BackendEvent{ - Key: store.Key{ + if v.mixerValidator != nil && un.GetAPIVersion() == mixerAPIVersion { + if !v.mixerValidator.SupportsKind(un.GetKind()) { + return fmt.Errorf("%s not implemented", un.GetKind()) + } + return v.mixerValidator.Validate(&mixerstore.BackendEvent{ + Type: mixerstore.Update, + Key: mixerstore.Key{ Name: un.GetName(), Namespace: un.GetNamespace(), Kind: un.GetKind(), }, - Value: mixerCrd.ToBackEndResource(un), - } - return mixerValidator.Validate(ev) - */ + Value: mixercrd.ToBackEndResource(un), + }) + } + return fmt.Errorf("%s.%s validation is not supported", un.GetKind(), un.GetAPIVersion()) } var errMissingResource = errors.New(`error: you must specify resources by --filename. @@ -136,6 +111,10 @@ func validateObjects(restClientGetter resource.RESTClientGetter, options resourc } var errs error + v := &validator{ + mixerValidator: mixervalidate.NewDefaultValidator(true), + } + _ = r.Visit(func(info *resource.Info, err error) error { if err != nil { return err @@ -154,13 +133,12 @@ func validateObjects(restClientGetter resource.RESTClientGetter, options resourc } } - if err := validateResource(un); err != nil { + if err := v.validateResource(un); err != nil { errs = multierror.Append(errs, fmt.Errorf("error validating resource (%v Name=%v Namespace=%v): %v", un.GetObjectKind().GroupVersionKind(), un.GetName(), un.GetNamespace(), err)) } return nil }) - if errs != nil { return errs } diff --git a/istioctl/pkg/validate/validate_test.go b/istioctl/pkg/validate/validate_test.go index 0592dcf86530..b8662fe112c7 100644 --- a/istioctl/pkg/validate/validate_test.go +++ b/istioctl/pkg/validate/validate_test.go @@ -24,6 +24,8 @@ import ( "github.com/ghodss/yaml" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + mixervalidate "istio.io/istio/mixer/pkg/validate" ) const ( @@ -140,18 +142,21 @@ func TestValidateResource(t *testing.T) { { name: "valid mixer configuration", in: validMixerRule, - valid: false, // TODO(https://github.com/istio/istio/issues/4887) + valid: true, }, { name: "invalid mixer configuration", in: invalidMixerRule, - valid: false, // TODO(https://github.com/istio/istio/issues/4887) + valid: false, }, } for i, c := range cases { t.Run(fmt.Sprintf("[%v] %v ", i, c.name), func(tt *testing.T) { - err := validateResource(fromYAML(c.in)) + v := &validator{ + mixerValidator: mixervalidate.NewDefaultValidator(false), + } + err := v.validateResource(fromYAML(c.in)) if (err == nil) != c.valid { tt.Fatalf("unexpected validation result: got %v want %v: err=%q", err == nil, c.valid, err) } diff --git a/mixer/adapter/bypass/bypass.go b/mixer/adapter/bypass/bypass.go index f0af1d82ae6b..2440c862ea29 100644 --- a/mixer/adapter/bypass/bypass.go +++ b/mixer/adapter/bypass/bypass.go @@ -30,6 +30,7 @@ import ( "istio.io/api/mixer/adapter/model/v1beta1" "istio.io/istio/mixer/adapter/bypass/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/checknothing" "istio.io/istio/mixer/template/metric" @@ -39,19 +40,9 @@ import ( // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "bypass", - Impl: "istio.io/istio/mixer/adapter/bypass", - Description: "Calls gRPC backends via the inline adapter model (useful for testing)", - SupportedTemplates: []string{ - checknothing.TemplateName, - reportnothing.TemplateName, - metric.TemplateName, - quota.TemplateName, - }, - DefaultConfig: &config.Params{}, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("bypass") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/circonus/circonus.go b/mixer/adapter/circonus/circonus.go index 337f1696d339..7107b6337aeb 100644 --- a/mixer/adapter/circonus/circonus.go +++ b/mixer/adapter/circonus/circonus.go @@ -30,6 +30,7 @@ import ( "github.com/circonus-labs/circonus-gometrics/checkmgr" "istio.io/istio/mixer/adapter/circonus/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/metric" ) @@ -193,15 +194,9 @@ func (h *handler) Close() error { // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "circonus", - Description: "Emit metrics to Circonus.com monitoring endpoint", - SupportedTemplates: []string{ - metric.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{SubmissionUrl: "", SubmissionInterval: 10 * time.Second}, - } + info := metadata.GetInfo("circonus") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } // logToEnvLogger converts CGM log package writes to env.Logger() diff --git a/mixer/adapter/cloudwatch/cloudwatch.go b/mixer/adapter/cloudwatch/cloudwatch.go index 015230041a48..0b14f3c8a470 100644 --- a/mixer/adapter/cloudwatch/cloudwatch.go +++ b/mixer/adapter/cloudwatch/cloudwatch.go @@ -28,6 +28,7 @@ import ( istio_policy_v1beta1 "istio.io/api/policy/v1beta1" "istio.io/istio/mixer/adapter/cloudwatch/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/logentry" "istio.io/istio/mixer/template/metric" @@ -197,14 +198,7 @@ func (h *handler) Close() error { // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "cloudwatch", - Description: "Sends metrics to cloudwatch and logs to cloudwatchlogs", - SupportedTemplates: []string{ - metric.TemplateName, - logentry.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{}, - } + info := metadata.GetInfo("cloudwatch") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/denier/denier.go b/mixer/adapter/denier/denier.go index d83e32609c70..7f26133805a3 100644 --- a/mixer/adapter/denier/denier.go +++ b/mixer/adapter/denier/denier.go @@ -30,6 +30,7 @@ import ( "github.com/gogo/googleapis/google/rpc" "istio.io/istio/mixer/adapter/denier/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/status" "istio.io/istio/mixer/template/checknothing" @@ -77,18 +78,9 @@ func (*handler) Close() error { return nil } // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "denier", - Impl: "istio.io/istio/mixer/adapter/denier", - Description: "Rejects any check and quota request with a configurable error", - SupportedTemplates: []string{ - checknothing.TemplateName, - listentry.TemplateName, - quota.TemplateName, - }, - DefaultConfig: defaultParam(), - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("denier") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/dogstatsd/dogstatsd.go b/mixer/adapter/dogstatsd/dogstatsd.go index 9eedfb94f971..a5469b2cae24 100644 --- a/mixer/adapter/dogstatsd/dogstatsd.go +++ b/mixer/adapter/dogstatsd/dogstatsd.go @@ -29,6 +29,7 @@ import ( descriptor "istio.io/api/policy/v1beta1" "istio.io/istio/mixer/adapter/dogstatsd/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/metric" ) @@ -201,18 +202,7 @@ func (h *handler) Close() error { return h.client.Close() } // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "dogstatsd", - Description: "Produces dogstatsd metrics", - SupportedTemplates: []string{ - metric.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{ - Address: "localhost:8125", - Prefix: "istio.", - BufferLength: 0, - GlobalTags: map[string]string{}, - }, - } + info := metadata.GetInfo("dogstatsd") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/fluentd/fluentd.go b/mixer/adapter/fluentd/fluentd.go index e05045417ea3..245fa5191697 100644 --- a/mixer/adapter/fluentd/fluentd.go +++ b/mixer/adapter/fluentd/fluentd.go @@ -33,6 +33,7 @@ import ( descriptor "istio.io/api/policy/v1beta1" "istio.io/istio/mixer/adapter/fluentd/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/logentry" ) @@ -43,10 +44,6 @@ const ( defaultTimeout = 1 * time.Minute ) -var ( - defaultAddress = "localhost:24224" -) - type ( builder struct { adpCfg *config.Params @@ -268,15 +265,7 @@ func (h *handler) Close() (err error) { // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "fluentd", - Description: "Sends logentrys to a fluentd instance", - SupportedTemplates: []string{ - logentry.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{ - Address: defaultAddress, - }, - } + info := metadata.GetInfo("fluentd") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/kubernetesenv/kubernetesenv.go b/mixer/adapter/kubernetesenv/kubernetesenv.go index 19bac3a8a339..312f36eba0fc 100644 --- a/mixer/adapter/kubernetesenv/kubernetesenv.go +++ b/mixer/adapter/kubernetesenv/kubernetesenv.go @@ -40,6 +40,7 @@ import ( "istio.io/istio/mixer/adapter/kubernetesenv/config" ktmpl "istio.io/istio/mixer/adapter/kubernetesenv/template" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/pkg/env" "istio.io/istio/pkg/kube/secretcontroller" @@ -94,17 +95,9 @@ var _ ktmpl.HandlerBuilder = &builder{} // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "kubernetesenv", - Impl: "istio.io/istio/mixer/adapter/kubernetesenv", - Description: "Provides platform specific functionality for the kubernetes environment", - SupportedTemplates: []string{ - ktmpl.TemplateName, - }, - DefaultConfig: conf, - - NewBuilder: func() adapter.HandlerBuilder { return newBuilder(newKubernetesClient) }, - } + info := metadata.GetInfo("kubernetesenv") + info.NewBuilder = func() adapter.HandlerBuilder { return newBuilder(newKubernetesClient) } + return info } func (b *builder) SetAdapterConfig(c adapter.Config) { diff --git a/mixer/adapter/list/list.go b/mixer/adapter/list/list.go index 61f41db2621e..890290f54ec4 100644 --- a/mixer/adapter/list/list.go +++ b/mixer/adapter/list/list.go @@ -37,6 +37,7 @@ import ( "istio.io/api/policy/v1beta1" "istio.io/istio/mixer/adapter/list/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/status" "istio.io/istio/mixer/template/listentry" @@ -285,22 +286,9 @@ func getCheckResult(config config.Params, code rpc.Code, msg string) adapter.Che // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "listchecker", - Impl: "istio.io/istio/mixer/adapter/list", - Description: "Checks whether an entry is present in a list", - SupportedTemplates: []string{listentry.TemplateName}, - DefaultConfig: &config.Params{ - RefreshInterval: 60 * time.Second, - Ttl: 300 * time.Second, - CachingInterval: 300 * time.Second, - CachingUseCount: 10000, - EntryType: config.STRINGS, - Blacklist: false, - }, - - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("listchecker") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/memquota/memquota.go b/mixer/adapter/memquota/memquota.go index 814993229433..96a79cdef075 100644 --- a/mixer/adapter/memquota/memquota.go +++ b/mixer/adapter/memquota/memquota.go @@ -35,6 +35,7 @@ import ( "time" "istio.io/istio/mixer/adapter/memquota/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/status" "istio.io/istio/mixer/template/quota" @@ -207,19 +208,9 @@ func (h *handler) Close() error { // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "memquota", - Impl: "istio.io/istio/mixer/adapter/memquota", - Description: "Volatile memory-based quota tracking", - SupportedTemplates: []string{ - quota.TemplateName, - }, - DefaultConfig: &config.Params{ - MinDeduplicationDuration: 1 * time.Second, - }, - - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("memquota") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/metadata/inventory.go b/mixer/adapter/metadata/inventory.go new file mode 100644 index 000000000000..14198c36801f --- /dev/null +++ b/mixer/adapter/metadata/inventory.go @@ -0,0 +1,360 @@ +// Copyright 2019 Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package metadata contains all compiled-in adapter metadata +package metadata + +import ( + "fmt" + "time" + + "github.com/gogo/googleapis/google/rpc" + "github.com/gogo/protobuf/types" + + bypass "istio.io/istio/mixer/adapter/bypass/config" + circonus "istio.io/istio/mixer/adapter/circonus/config" + cloudwatch "istio.io/istio/mixer/adapter/cloudwatch/config" + denier "istio.io/istio/mixer/adapter/denier/config" + dogstatsd "istio.io/istio/mixer/adapter/dogstatsd/config" + fluentd "istio.io/istio/mixer/adapter/fluentd/config" + kubernetesenv "istio.io/istio/mixer/adapter/kubernetesenv/config" + list "istio.io/istio/mixer/adapter/list/config" + memquota "istio.io/istio/mixer/adapter/memquota/config" + opa "istio.io/istio/mixer/adapter/opa/config" + prometheus "istio.io/istio/mixer/adapter/prometheus/config" + rbac "istio.io/istio/mixer/adapter/rbac/config" + redisquota "istio.io/istio/mixer/adapter/redisquota/config" + signalfx "istio.io/istio/mixer/adapter/signalfx/config" + solarwinds "istio.io/istio/mixer/adapter/solarwinds/config" + stackdriver "istio.io/istio/mixer/adapter/stackdriver/config" + statsd "istio.io/istio/mixer/adapter/statsd/config" + stdio "istio.io/istio/mixer/adapter/stdio/config" + zipkin "istio.io/istio/mixer/adapter/zipkin/config" + + ktmpl "istio.io/istio/mixer/adapter/kubernetesenv/template" + "istio.io/istio/mixer/pkg/adapter" + "istio.io/istio/mixer/pkg/status" + "istio.io/istio/mixer/template/authorization" + "istio.io/istio/mixer/template/checknothing" + edgepb "istio.io/istio/mixer/template/edge" + "istio.io/istio/mixer/template/listentry" + "istio.io/istio/mixer/template/logentry" + "istio.io/istio/mixer/template/metric" + "istio.io/istio/mixer/template/quota" + "istio.io/istio/mixer/template/reportnothing" + "istio.io/istio/mixer/template/tracespan" +) + +var ( + Infos = []adapter.Info{ + { + Name: "bypass", + Impl: "istio.io/istio/mixer/adapter/bypass", + Description: "Calls gRPC backends via the inline adapter model (useful for testing)", + SupportedTemplates: []string{ + checknothing.TemplateName, + reportnothing.TemplateName, + metric.TemplateName, + quota.TemplateName, + }, + DefaultConfig: &bypass.Params{}, + }, + + { + Name: "circonus", + Description: "Emit metrics to Circonus.com monitoring endpoint", + SupportedTemplates: []string{ + metric.TemplateName, + }, + DefaultConfig: &circonus.Params{SubmissionUrl: "", SubmissionInterval: 10 * time.Second}, + }, + + { + Name: "cloudwatch", + Description: "Sends metrics to cloudwatch and logs to cloudwatchlogs", + SupportedTemplates: []string{ + metric.TemplateName, + logentry.TemplateName, + }, + DefaultConfig: &cloudwatch.Params{}, + }, + + { + Name: "denier", + Impl: "istio.io/istio/mixer/adapter/denier", + Description: "Rejects any check and quota request with a configurable error", + SupportedTemplates: []string{ + checknothing.TemplateName, + listentry.TemplateName, + quota.TemplateName, + }, + DefaultConfig: &denier.Params{ + Status: status.New(rpc.FAILED_PRECONDITION), + ValidDuration: 5 * time.Second, + ValidUseCount: 1000, + }, + }, + + { + Name: "dogstatsd", + Description: "Produces dogstatsd metrics", + SupportedTemplates: []string{ + metric.TemplateName, + }, + DefaultConfig: &dogstatsd.Params{ + Address: "localhost:8125", + Prefix: "istio.", + BufferLength: 0, + GlobalTags: map[string]string{}, + }, + }, + + { + Name: "fluentd", + Description: "Sends logentrys to a fluentd instance", + SupportedTemplates: []string{ + logentry.TemplateName, + }, + DefaultConfig: &fluentd.Params{ + Address: "localhost:24224", + }, + }, + + { + Name: "kubernetesenv", + Impl: "istio.io/istio/mixer/adapter/kubernetesenv", + Description: "Provides platform specific functionality for the kubernetes environment", + SupportedTemplates: []string{ + ktmpl.TemplateName, + }, + DefaultConfig: &kubernetesenv.Params{ + KubeconfigPath: "", + // k8s cache invalidation + // TODO: determine a reasonable default + CacheRefreshDuration: 5 * time.Minute, + ClusterRegistriesNamespace: "", + }, + }, + + { + Name: "listchecker", + Impl: "istio.io/istio/mixer/adapter/list", + Description: "Checks whether an entry is present in a list", + SupportedTemplates: []string{listentry.TemplateName}, + DefaultConfig: &list.Params{ + RefreshInterval: 60 * time.Second, + Ttl: 300 * time.Second, + CachingInterval: 300 * time.Second, + CachingUseCount: 10000, + EntryType: list.STRINGS, + Blacklist: false, + }, + }, + + { + Name: "memquota", + Impl: "istio.io/istio/mixer/adapter/memquota", + Description: "Volatile memory-based quota tracking", + SupportedTemplates: []string{ + quota.TemplateName, + }, + DefaultConfig: &memquota.Params{ + MinDeduplicationDuration: 1 * time.Second, + }, + }, + + { + Name: "noop", + Impl: "istio.io/istio/mixer/adapter/noop", + Description: "Does nothing (useful for testing)", + SupportedTemplates: []string{ + authorization.TemplateName, + checknothing.TemplateName, + reportnothing.TemplateName, + listentry.TemplateName, + logentry.TemplateName, + metric.TemplateName, + quota.TemplateName, + tracespan.TemplateName, + }, + DefaultConfig: &types.Empty{}, + }, + + { + Name: "opa", + Impl: "istio.io/istio/mixer/adapter/opa", + Description: "Istio Authorization with Open Policy Agent engine", + SupportedTemplates: []string{ + authorization.TemplateName, + }, + DefaultConfig: &opa.Params{}, + }, + + { + Name: "prometheus", + Impl: "istio.io/istio/mixer/adapter/prometheus", + Description: "Publishes prometheus metrics", + SupportedTemplates: []string{ + metric.TemplateName, + }, + DefaultConfig: &prometheus.Params{}, + }, + + { + Name: "rbac", + Impl: "istio.io/istio/mixer/adapter/rbac", + Description: "Mixer RBAC adapter is deprecated by native RBAC implemented in Envoy proxy. See " + + "https://istio.io/docs/concepts/security/#enabling-authorization for enabling the native RBAC with " + + "your existing service role and service role binding.", + SupportedTemplates: []string{ + authorization.TemplateName, + }, + DefaultConfig: &rbac.Params{}, + }, + + { + Name: "redisquota", + Impl: "istio.io/mixer/adapter/redisquota", + Description: "Redis-based quotas.", + SupportedTemplates: []string{ + quota.TemplateName, + }, + DefaultConfig: &redisquota.Params{ + RedisServerUrl: "localhost:6379", + ConnectionPoolSize: 10, + }, + }, + + { + Name: "signalfx", + Description: "Sends metrics and traces to SignalFx", + SupportedTemplates: []string{ + metric.TemplateName, + tracespan.TemplateName, + }, + DefaultConfig: &signalfx.Params{ + EnableMetrics: true, + EnableTracing: true, + DatapointInterval: 10 * time.Second, + TracingBufferSize: 1000, + TracingSampleProbability: 1.0, + }, + }, + + { + Name: "solarwinds", + Impl: "istio.io/istio/mixer/adapter/solarwinds", + Description: "Publishes metrics to appoptics and logs to papertrail", + SupportedTemplates: []string{ + metric.TemplateName, + logentry.TemplateName, + }, + DefaultConfig: &solarwinds.Params{}, + }, + + { + Name: "stackdriver", + Impl: "istio.io/istio/mixer/adapter/stackdriver", + Description: "Publishes StackDriver metrics, logs and traces.", + SupportedTemplates: []string{ + edgepb.TemplateName, + logentry.TemplateName, + metric.TemplateName, + tracespan.TemplateName, + }, + DefaultConfig: &stackdriver.Params{}, + }, + + { + Name: "statsd", + Impl: "istio.io/istio/mixer/adapter/statsd", + Description: "Produces statsd metrics", + SupportedTemplates: []string{ + metric.TemplateName, + }, + DefaultConfig: &statsd.Params{ + Address: "localhost:8125", + Prefix: "", + FlushDuration: 300 * time.Millisecond, + FlushBytes: 512, + SamplingRate: 1.0, + }, + }, + + { + Name: "stdio", + Impl: "istio.io/istio/mixer/adapter/stdio", + Description: "Writes logs and metrics to a standard I/O stream", + SupportedTemplates: []string{ + logentry.TemplateName, + metric.TemplateName, + }, + DefaultConfig: &stdio.Params{ + LogStream: stdio.STDOUT, + MetricLevel: stdio.INFO, + OutputLevel: stdio.INFO, + OutputAsJson: true, + MaxDaysBeforeRotation: 30, + MaxMegabytesBeforeRotation: 100 * 1024 * 1024, + MaxRotatedFiles: 1000, + SeverityLevels: map[string]stdio.Params_Level{ + "INFORMATIONAL": stdio.INFO, + "informational": stdio.INFO, + "INFO": stdio.INFO, + "info": stdio.INFO, + "WARNING": stdio.WARNING, + "warning": stdio.WARNING, + "WARN": stdio.WARNING, + "warn": stdio.WARNING, + "ERROR": stdio.ERROR, + "error": stdio.ERROR, + "ERR": stdio.ERROR, + "err": stdio.ERROR, + "FATAL": stdio.ERROR, + "fatal": stdio.ERROR, + }, + }, + }, + + { + Name: "zipkin", + Impl: "istio.io/istio/mixer/adapter/zipkin", + Description: "Publishes traces to Zipkin.", + SupportedTemplates: []string{ + tracespan.TemplateName, + }, + DefaultConfig: &zipkin.Params{}, + }, + } +) + +// GetInfo looks up an adapter info from the declaration list by name +func GetInfo(name string) adapter.Info { + for _, info := range Infos { + if info.Name == name { + return info + } + } + panic(fmt.Errorf("requesting a missing descriptor %q", name)) +} + +// InfoMap returns an indexed map of adapter infos. +func InfoMap() map[string]*adapter.Info { + out := make(map[string]*adapter.Info, len(Infos)) + for i := range Infos { + info := Infos[i] + out[info.Name] = &info + } + return out +} diff --git a/mixer/adapter/noop/noop.go b/mixer/adapter/noop/noop.go index bfdd56ad8702..8ffb527d28a2 100644 --- a/mixer/adapter/noop/noop.go +++ b/mixer/adapter/noop/noop.go @@ -22,8 +22,8 @@ import ( "time" "github.com/gogo/googleapis/google/rpc" - "github.com/gogo/protobuf/types" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/authorization" "istio.io/istio/mixer/template/checknothing" @@ -85,24 +85,9 @@ func (*handler) Close() error { return nil } // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "noop", - Impl: "istio.io/istio/mixer/adapter/noop", - Description: "Does nothing (useful for testing)", - SupportedTemplates: []string{ - authorization.TemplateName, - checknothing.TemplateName, - reportnothing.TemplateName, - listentry.TemplateName, - logentry.TemplateName, - metric.TemplateName, - quota.TemplateName, - tracespan.TemplateName, - }, - DefaultConfig: &types.Empty{}, - - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("noop") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct{} diff --git a/mixer/adapter/opa/opa.go b/mixer/adapter/opa/opa.go index 0e843b9c5e13..6804d8461e38 100644 --- a/mixer/adapter/opa/opa.go +++ b/mixer/adapter/opa/opa.go @@ -24,6 +24,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/rego" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/opa/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/status" @@ -253,14 +254,7 @@ func (h *handler) Close() error { // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "opa", - Impl: "istio.io/istio/mixer/adapter/opa", - Description: "Istio Authorization with Open Policy Agent engine", - SupportedTemplates: []string{ - authorization.TemplateName, - }, - DefaultConfig: &config.Params{}, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("opa") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/prometheus/prometheus.go b/mixer/adapter/prometheus/prometheus.go index c47f04b9db45..57bfd407ed75 100644 --- a/mixer/adapter/prometheus/prometheus.go +++ b/mixer/adapter/prometheus/prometheus.go @@ -35,6 +35,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/prometheus/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/pool" @@ -91,16 +92,9 @@ func GetInfoWithAddr(addr string) (adapter.Info, Server) { srv: newServer(addr), } singletonBuilder.clearState() - return adapter.Info{ - Name: "prometheus", - Impl: "istio.io/istio/mixer/adapter/prometheus", - Description: "Publishes prometheus metrics", - SupportedTemplates: []string{ - metric.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return singletonBuilder }, - DefaultConfig: &config.Params{}, - }, singletonBuilder.srv + info := metadata.GetInfo("prometheus") + info.NewBuilder = func() adapter.HandlerBuilder { return singletonBuilder } + return info, singletonBuilder.srv } // GetInfo returns the Info associated with this adapter. diff --git a/mixer/adapter/rbac/rbac.go b/mixer/adapter/rbac/rbac.go index 90bd1878414c..6bfd7f114406 100644 --- a/mixer/adapter/rbac/rbac.go +++ b/mixer/adapter/rbac/rbac.go @@ -21,7 +21,7 @@ package rbac import ( "context" - "istio.io/istio/mixer/adapter/rbac/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/authorization" ) @@ -54,14 +54,7 @@ func (b *builder) Build(ctx context.Context, env adapter.Env) (adapter.Handler, // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "rbac", - Impl: "istio.io/istio/mixer/adapter/rbac", - Description: prompt, - SupportedTemplates: []string{ - authorization.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{}, - } + info := metadata.GetInfo("rbac") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/redisquota/redisquota.go b/mixer/adapter/redisquota/redisquota.go index 3779380e6470..cef7e905d563 100644 --- a/mixer/adapter/redisquota/redisquota.go +++ b/mixer/adapter/redisquota/redisquota.go @@ -31,6 +31,7 @@ import ( "github.com/go-redis/redis" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/redisquota/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/status" @@ -375,19 +376,9 @@ func (h handler) Close() error { // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "redisquota", - Impl: "istio.io/mixer/adapter/redisquota", - Description: "Redis-based quotas.", - SupportedTemplates: []string{ - quota.TemplateName, - }, - DefaultConfig: &config.Params{ - RedisServerUrl: "localhost:6379", - ConnectionPoolSize: 10, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("redisquota") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } /////////////////////////////////////////////////////// diff --git a/mixer/adapter/signalfx/signalfx.go b/mixer/adapter/signalfx/signalfx.go index 8e7b50a65848..b8e42ef76728 100644 --- a/mixer/adapter/signalfx/signalfx.go +++ b/mixer/adapter/signalfx/signalfx.go @@ -27,6 +27,7 @@ import ( "github.com/signalfx/golib/sfxclient" "istio.io/api/policy/v1beta1" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/signalfx/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/metric" @@ -202,20 +203,7 @@ func (h *handler) Close() error { // GetInfo returns the adapter.Info specific to this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "signalfx", - Description: "Sends metrics and traces to SignalFx", - SupportedTemplates: []string{ - metric.TemplateName, - tracespan.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{ - EnableMetrics: true, - EnableTracing: true, - DatapointInterval: 10 * time.Second, - TracingBufferSize: 1000, - TracingSampleProbability: 1.0, - }, - } + info := metadata.GetInfo("signalfx") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } diff --git a/mixer/adapter/solarwinds/solarwinds.go b/mixer/adapter/solarwinds/solarwinds.go index 33ae482fa916..08a16ad96695 100644 --- a/mixer/adapter/solarwinds/solarwinds.go +++ b/mixer/adapter/solarwinds/solarwinds.go @@ -24,6 +24,7 @@ import ( "fmt" "regexp" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/solarwinds/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/logentry" @@ -53,17 +54,9 @@ var ( // GetInfo returns the Info associated with this adapter. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "solarwinds", - Impl: "istio.io/istio/mixer/adapter/solarwinds", - Description: "Publishes metrics to appoptics and logs to papertrail", - SupportedTemplates: []string{ - metric.TemplateName, - logentry.TemplateName, - }, - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - DefaultConfig: &config.Params{}, - } + info := metadata.GetInfo("solarwinds") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } func (b *builder) SetAdapterConfig(cfg adapter.Config) { diff --git a/mixer/adapter/stackdriver/stackdriver.go b/mixer/adapter/stackdriver/stackdriver.go index c03d88c43eb6..0333ddf6cced 100644 --- a/mixer/adapter/stackdriver/stackdriver.go +++ b/mixer/adapter/stackdriver/stackdriver.go @@ -25,7 +25,7 @@ import ( md "cloud.google.com/go/compute/metadata" multierror "github.com/hashicorp/go-multierror" - "istio.io/istio/mixer/adapter/stackdriver/config" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/stackdriver/contextgraph" "istio.io/istio/mixer/adapter/stackdriver/helper" "istio.io/istio/mixer/adapter/stackdriver/log" @@ -85,25 +85,16 @@ func GetInfo() adapter.Info { return md.Zone() } mg := helper.NewMetadataGenerator(md.OnGCE, md.ProjectID, clusterLocationFn, clusterNameFn) - return adapter.Info{ - Name: "stackdriver", - Impl: "istio.io/istio/mixer/adapter/stackdriver", - Description: "Publishes StackDriver metrics, logs and traces.", - SupportedTemplates: []string{ - edgepb.TemplateName, - logentry.TemplateName, - metric.TemplateName, - tracespan.TemplateName, - }, - DefaultConfig: &config.Params{}, - NewBuilder: func() adapter.HandlerBuilder { - return &builder{ - m: sdmetric.NewBuilder(mg), - l: log.NewBuilder(mg), - t: trace.NewBuilder(mg), - c: contextgraph.NewBuilder(mg), - } - }} + info := metadata.GetInfo("stackdriver") + info.NewBuilder = func() adapter.HandlerBuilder { + return &builder{ + m: sdmetric.NewBuilder(mg), + l: log.NewBuilder(mg), + t: trace.NewBuilder(mg), + c: contextgraph.NewBuilder(mg), + } + } + return info } func (b *builder) SetMetricTypes(metrics map[string]*metric.Type) { diff --git a/mixer/adapter/statsd/statsd.go b/mixer/adapter/statsd/statsd.go index c308dc8bee09..381460af21b4 100644 --- a/mixer/adapter/statsd/statsd.go +++ b/mixer/adapter/statsd/statsd.go @@ -29,6 +29,7 @@ import ( "github.com/cactus/go-statsd-client/statsd" multierror "github.com/hashicorp/go-multierror" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/statsd/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/pool" @@ -115,23 +116,9 @@ func (h *handler) Close() error { return h.client.Close() } // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "statsd", - Impl: "istio.io/istio/mixer/adapter/statsd", - Description: "Produces statsd metrics", - SupportedTemplates: []string{ - metric.TemplateName, - }, - DefaultConfig: &config.Params{ - Address: "localhost:8125", - Prefix: "", - FlushDuration: 300 * time.Millisecond, - FlushBytes: 512, - SamplingRate: 1.0, - }, - - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("statsd") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/stdio/stdio.go b/mixer/adapter/stdio/stdio.go index c8e23a366fbc..c1d345326d98 100644 --- a/mixer/adapter/stdio/stdio.go +++ b/mixer/adapter/stdio/stdio.go @@ -31,6 +31,7 @@ import ( "go.uber.org/zap/zapcore" istio_policy_v1beta1 "istio.io/api/policy/v1beta1" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/stdio/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/template/logentry" @@ -138,42 +139,9 @@ func convertValueTypes(value interface{}, varName string, logEntryTypes map[stri // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "stdio", - Impl: "istio.io/istio/mixer/adapter/stdio", - Description: "Writes logs and metrics to a standard I/O stream", - SupportedTemplates: []string{ - logentry.TemplateName, - metric.TemplateName, - }, - DefaultConfig: &config.Params{ - LogStream: config.STDOUT, - MetricLevel: config.INFO, - OutputLevel: config.INFO, - OutputAsJson: true, - MaxDaysBeforeRotation: 30, - MaxMegabytesBeforeRotation: 100 * 1024 * 1024, - MaxRotatedFiles: 1000, - SeverityLevels: map[string]config.Params_Level{ - "INFORMATIONAL": config.INFO, - "informational": config.INFO, - "INFO": config.INFO, - "info": config.INFO, - "WARNING": config.WARNING, - "warning": config.WARNING, - "WARN": config.WARNING, - "warn": config.WARNING, - "ERROR": config.ERROR, - "error": config.ERROR, - "ERR": config.ERROR, - "err": config.ERROR, - "FATAL": config.ERROR, - "fatal": config.ERROR, - }, - }, - - NewBuilder: func() adapter.HandlerBuilder { return &builder{} }, - } + info := metadata.GetInfo("stdio") + info.NewBuilder = func() adapter.HandlerBuilder { return &builder{} } + return info } type builder struct { diff --git a/mixer/adapter/zipkin/zipkin.go b/mixer/adapter/zipkin/zipkin.go index 921206c57c78..e6f333cc8e88 100644 --- a/mixer/adapter/zipkin/zipkin.go +++ b/mixer/adapter/zipkin/zipkin.go @@ -26,6 +26,7 @@ import ( oczipkin "go.opencensus.io/exporter/zipkin" "go.opencensus.io/trace" + "istio.io/istio/mixer/adapter/metadata" "istio.io/istio/mixer/adapter/zipkin/config" "istio.io/istio/mixer/pkg/adapter" "istio.io/istio/mixer/pkg/adapter/opencensus" @@ -44,17 +45,11 @@ var _ tracespan.HandlerBuilder = &builder{} // GetInfo returns the Info associated with this adapter implementation. func GetInfo() adapter.Info { - return adapter.Info{ - Name: "zipkin", - Impl: "istio.io/istio/mixer/adapter/zipkin", - Description: "Publishes traces to Zipkin.", - SupportedTemplates: []string{ - tracespan.TemplateName, - }, - DefaultConfig: &config.Params{}, - NewBuilder: func() adapter.HandlerBuilder { - return &builder{} - }} + info := metadata.GetInfo("zipkin") + info.NewBuilder = func() adapter.HandlerBuilder { + return &builder{} + } + return info } func (b *builder) SetTraceSpanTypes(types map[string]*tracespan.Type) { diff --git a/mixer/pkg/config/store/convert.go b/mixer/pkg/config/store/convert.go index 9e1c633001f4..4c9758780efc 100644 --- a/mixer/pkg/config/store/convert.go +++ b/mixer/pkg/config/store/convert.go @@ -25,28 +25,6 @@ import ( "istio.io/istio/pkg/log" ) -const ( - ruleKind = "rule" - selectorField = "selector" - matchField = "match" -) - -// warnDeprecationAndFix warns users about deprecated fields. -// It maps the field into new name. -func warnDeprecationAndFix(key Key, spec map[string]interface{}) map[string]interface{} { - if key.Kind != ruleKind { - return spec - } - sel := spec[selectorField] - if sel == nil { - return spec - } - log.Warnf("Deprecated field 'selector' used in %s. Use 'match' instead.", key) - spec[matchField] = sel - delete(spec, selectorField) - return spec -} - // cloneMessage looks up the kind in the map, and creates a clone of it. func cloneMessage(kind string, kinds map[string]proto.Message) (proto.Message, error) { msg, ok := kinds[kind] @@ -58,7 +36,7 @@ func cloneMessage(kind string, kinds map[string]proto.Message) (proto.Message, e // convert converts unstructured spec into the target proto. func convert(key Key, spec map[string]interface{}, target proto.Message) error { - jsonData, err := json.Marshal(warnDeprecationAndFix(key, spec)) + jsonData, err := json.Marshal(spec) if err != nil { return err } @@ -68,3 +46,24 @@ func convert(key Key, spec map[string]interface{}, target proto.Message) error { return err } + +// ConvertValue from JSON using a protobuf mapping +func ConvertValue(ev BackendEvent, kinds map[string]proto.Message) (Event, error) { + pbSpec, err := cloneMessage(ev.Kind, kinds) + if err != nil { + return Event{}, err + } + if ev.Value == nil { + return Event{Key: ev.Key, Type: ev.Type}, nil + } + if err = convert(ev.Key, ev.Value.Spec, pbSpec); err != nil { + return Event{}, err + } + return Event{ + Key: ev.Key, + Type: ev.Type, + Value: &Resource{ + Metadata: ev.Value.Metadata, + Spec: pbSpec, + }}, nil +} diff --git a/mixer/pkg/config/store/convert_test.go b/mixer/pkg/config/store/convert_test.go index 8ffd88438b2b..49187d8642ac 100644 --- a/mixer/pkg/config/store/convert_test.go +++ b/mixer/pkg/config/store/convert_test.go @@ -50,6 +50,21 @@ func TestConvert(t *testing.T) { t.Errorf("%s: Got %+v, Want %+v", tt.title, tt.dest, tt.expected) } } + + ev := BackendEvent{ + Key: Key{ + Kind: "handler", + }, + } + want := Event{ + Key: Key{ + Kind: "handler", + }, + } + + if got, err := ConvertValue(ev, map[string]proto.Message{"handler": &cfg.Handler{}}); err != nil || !reflect.DeepEqual(got, want) { + t.Errorf("ConvertValue(%#v) => got %#v, %v, want %#v", ev, got, err, want) + } } func TestConvertFail(t *testing.T) { diff --git a/mixer/pkg/config/store/queue.go b/mixer/pkg/config/store/queue.go index 8a1288460eb0..0c166abf6ef0 100644 --- a/mixer/pkg/config/store/queue.go +++ b/mixer/pkg/config/store/queue.go @@ -41,26 +41,6 @@ func newQueue(chin <-chan BackendEvent, kinds map[string]proto.Message) *eventQu return eq } -func (q *eventQueue) convertValue(ev BackendEvent) (Event, error) { - pbSpec, err := cloneMessage(ev.Kind, q.kinds) - if err != nil { - return Event{}, err - } - if ev.Value == nil { - return Event{Key: ev.Key, Type: ev.Type}, nil - } - if err = convert(ev.Key, ev.Value.Spec, pbSpec); err != nil { - return Event{}, err - } - return Event{ - Key: ev.Key, - Type: ev.Type, - Value: &Resource{ - Metadata: ev.Value.Metadata, - Spec: pbSpec, - }}, nil -} - func (q *eventQueue) run() { loop: for { @@ -68,7 +48,7 @@ loop: case <-q.closec: break loop case ev := <-q.chin: - converted, err := q.convertValue(ev) + converted, err := ConvertValue(ev, q.kinds) if err != nil { log.Errorf("Failed to convert %s an event: %v", ev.Key, err) break @@ -79,7 +59,7 @@ loop: case <-q.closec: break loop case ev := <-q.chin: - converted, err = q.convertValue(ev) + converted, err = ConvertValue(ev, q.kinds) if err != nil { log.Errorf("Failed to convert %s an event: %v", ev.Key, err) break diff --git a/mixer/pkg/config/store/store.go b/mixer/pkg/config/store/store.go index 6c0c414fac47..0b1ca3338b70 100644 --- a/mixer/pkg/config/store/store.go +++ b/mixer/pkg/config/store/store.go @@ -110,6 +110,7 @@ type Event struct { // BackendValidator defines the interface to validte unstructured event. type BackendValidator interface { Validate(ev *BackendEvent) error + SupportsKind(string) bool } // Validator defines the interface to validate a new change. diff --git a/mixer/pkg/config/store/validator.go b/mixer/pkg/config/store/validator.go deleted file mode 100644 index 571baf862d8d..000000000000 --- a/mixer/pkg/config/store/validator.go +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2017 Istio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package store - -import ( - "errors" - - "github.com/gogo/protobuf/proto" - - "istio.io/istio/pkg/log" -) - -type perKindValidateFunc func(br *BackEndResource) error - -type perKindValidator struct { - pbSpec proto.Message - validate perKindValidateFunc -} - -func (pv *perKindValidator) validateAndConvert(key Key, br *BackEndResource, res *Resource) error { - if pv.validate != nil { - if err := pv.validate(br); err != nil { - return err - } - } - res.Spec = proto.Clone(pv.pbSpec) - return convert(key, br.Spec, res.Spec) -} - -func validateRule(br *BackEndResource) error { - _, matchExists := br.Spec[matchField] - _, selectorExists := br.Spec[selectorField] - if !matchExists && selectorExists { - return errors.New("field 'selector' is deprecated, use 'match' instead") - } - if selectorExists { - log.Warnf("Deprecated field 'selector' used in %s. Use 'match' instead.", br.Metadata.Name) - } - return nil -} - -// validator provides the default structural validation with delegating -// an external validator for the referential integrity. -type validator struct { - externalValidator Validator - perKindValidators map[string]*perKindValidator -} - -// NewValidator creates a default validator which validates the structure through registered -// kinds and referential integrity through ev. -func NewValidator(ev Validator, kinds map[string]proto.Message) BackendValidator { - vs := make(map[string]*perKindValidator, len(kinds)) - for k, pb := range kinds { - var validateFunc perKindValidateFunc - if k == ruleKind { - validateFunc = validateRule - } - vs[k] = &perKindValidator{pb, validateFunc} - } - return &validator{ - externalValidator: ev, - perKindValidators: vs, - } -} - -func (v *validator) Validate(bev *BackendEvent) error { - pkv, ok := v.perKindValidators[bev.Key.Kind] - if !ok { - // Pass unrecognized kinds -- they should be validated by somewhere else. - log.Debugf("unrecognized kind %s is requested to validate", bev.Key.Kind) - return nil - } - ev := &Event{ - Type: bev.Type, - Key: bev.Key, - } - if bev.Type == Update { - ev.Value = &Resource{Metadata: bev.Value.Metadata} - if err := pkv.validateAndConvert(bev.Key, bev.Value, ev.Value); err != nil { - return err - } - } - if v.externalValidator == nil { - return nil - } - return v.externalValidator.Validate(ev) -} diff --git a/mixer/pkg/config/store/validator_test.go b/mixer/pkg/config/store/validator_test.go deleted file mode 100644 index 6ddb7fe4b6e2..000000000000 --- a/mixer/pkg/config/store/validator_test.go +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2017 Istio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package store - -import ( - "errors" - "strings" - "testing" - - "github.com/gogo/protobuf/proto" - - cfg "istio.io/api/policy/v1beta1" -) - -type fakeValidator struct { - err error -} - -func (v *fakeValidator) Validate(*Event) error { - return v.err -} - -func backendEvent(t ChangeType, kind, name string, spec map[string]interface{}) *BackendEvent { - namespace := "ns" - return &BackendEvent{ - Type: t, - Key: Key{Kind: kind, Namespace: namespace, Name: name}, - Value: &BackEndResource{ - Metadata: ResourceMeta{ - Name: name, - Namespace: namespace, - }, - Spec: spec, - }, - } -} - -func TestValidate(t *testing.T) { - for _, c := range []struct { - title string - kinds map[string]proto.Message - externalError error - ev *BackendEvent - want error - }{ - { - "update", - map[string]proto.Message{ - "Handler": &cfg.Handler{}, - }, - nil, - backendEvent(Update, "Handler", "foo", map[string]interface{}{"adapter": "noop", "name": "default"}), - nil, - }, - { - "delete", - map[string]proto.Message{ - "Handler": &cfg.Handler{}, - }, - nil, - backendEvent(Delete, "Handler", "foo", nil), - nil, - }, - { - "unknown kinds", - map[string]proto.Message{}, - errors.New("fail"), - backendEvent(Update, "Unknown", "bar", map[string]interface{}{"foo": "bar"}), - nil, - }, - { - "external validator failures", - map[string]proto.Message{"Handler": &cfg.Handler{}}, - errors.New("external validator failure"), - backendEvent(Update, "Handler", "foo", map[string]interface{}{"adapter": "noop", "name": "default"}), - errors.New("external validator failure"), - }, - { - "external validator failures on delete", - map[string]proto.Message{"Handler": &cfg.Handler{}}, - errors.New("external validator failure"), - backendEvent(Delete, "Handler", "foo", nil), - errors.New("external validator failure"), - }, - { - "deprecated field", - map[string]proto.Message{ruleKind: &cfg.Rule{}}, - nil, - backendEvent(Update, ruleKind, "foo", map[string]interface{}{"selector": "selector"}), - errors.New("field 'selector' is deprecated"), - }, - } { - t.Run(c.title, func(tt *testing.T) { - v := NewValidator(&fakeValidator{c.externalError}, c.kinds) - err := v.Validate(c.ev) - if c.want == nil && err != nil { - tt.Errorf("Got %v, Want nil", err) - } else if c.want != nil && !strings.Contains(err.Error(), c.want.Error()) { - tt.Errorf("Got %v, Want to contain %s", err, c.want.Error()) - } - }) - } -} diff --git a/mixer/pkg/runtime/config/ephemeral.go b/mixer/pkg/runtime/config/ephemeral.go index cb0855e11eaf..48d91c8b3e8f 100644 --- a/mixer/pkg/runtime/config/ephemeral.go +++ b/mixer/pkg/runtime/config/ephemeral.go @@ -193,7 +193,6 @@ func (e *Ephemeral) BuildSnapshot() (*Snapshot, error) { } e.lock.RUnlock() - log.Infof("Built new config.Snapshot: id='%d'", id) log.Debugf("config.Snapshot creation error=%v, contents:\n%s", errs.ErrorOrNil(), s) return s, errs.ErrorOrNil() } @@ -893,7 +892,7 @@ func (e *Ephemeral) processDynamicTemplateConfigs(ctx context.Context, errs *mul func appendErr(ctx context.Context, errs *multierror.Error, field string, measure *stats.Int64Measure, format string, a ...interface{}) { err := fmt.Errorf(format, a...) - log.Error(err.Error()) + log.Debug(err.Error()) stats.Record(ctx, measure.M(1)) _ = multierror.Append(errs, adapter.ConfigError{Field: field, Underlying: err}) } diff --git a/mixer/pkg/runtime/config/validator/validator.go b/mixer/pkg/runtime/config/validator/validator.go deleted file mode 100644 index 880c77fa18e5..000000000000 --- a/mixer/pkg/runtime/config/validator/validator.go +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright 2017 Istio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package validator - -import ( - "fmt" - "strings" - "time" - - "github.com/gogo/protobuf/proto" - multierror "github.com/hashicorp/go-multierror" - - cpb "istio.io/api/policy/v1beta1" - "istio.io/istio/mixer/pkg/adapter" - "istio.io/istio/mixer/pkg/config/store" - "istio.io/istio/mixer/pkg/runtime/config" - "istio.io/istio/mixer/pkg/runtime/config/constant" - "istio.io/istio/mixer/pkg/template" -) - -// Validator offers semantic validation of the config changes. -type Validator struct { - handlerBuilders map[string]adapter.HandlerBuilder - templates map[string]*template.Info - donec chan struct{} - e *config.Ephemeral -} - -// NewValidator creates a new store.Validator instance which validates runtime semantics of -// the configs. -func NewValidator(s store.Store, - adapterInfo map[string]*adapter.Info, templateInfo map[string]*template.Info) (store.Validator, error) { - kinds := config.KindMap(adapterInfo, templateInfo) - if err := s.Init(kinds); err != nil { - return nil, err - } - data, ch, err := store.StartWatch(s) - if err != nil { - return nil, err - } - hb := make(map[string]adapter.HandlerBuilder, len(adapterInfo)) - for k, ai := range adapterInfo { - hb[k] = ai.NewBuilder() - } - configData := make(map[store.Key]proto.Message, len(data)) - manifests := map[store.Key]*cpb.AttributeManifest{} - for k, obj := range data { - if k.Kind == constant.AttributeManifestKind { - manifests[k] = obj.Spec.(*cpb.AttributeManifest) - } - configData[k] = obj.Spec - } - e := config.NewEphemeral(templateInfo, adapterInfo) - v := &Validator{ - handlerBuilders: hb, - templates: templateInfo, - donec: make(chan struct{}), - e: e, - } - v.e.SetState(data) - go store.WatchChanges(ch, v.donec, time.Second, v.e.ApplyEvent) - return v, nil -} - -// Stop stops the validator. -func (v *Validator) Stop() { - close(v.donec) -} - -// Validate implements store.Validator interface. -func (v *Validator) Validate(ev *store.Event) error { - // get old state so we can revert in case of validation error. - oldEntryVal, exists := v.e.GetEntry(ev) - - v.e.ApplyEvent([]*store.Event{ev}) - s, err := v.e.BuildSnapshot() - // now validate snapshot as a whole - if err == nil { - err = validateHandlers(s) - } - - if err != nil { - reverseEvent := *ev - if exists { - reverseEvent.Value = oldEntryVal - reverseEvent.Type = store.Update - } else if ev.Type == store.Update { // didn't existed before. - reverseEvent.Type = store.Delete - } - v.e.ApplyEvent([]*store.Event{&reverseEvent}) - } - - return err -} - -func validateHandlers(s *config.Snapshot) error { - var errs error - instancesByHandler := config.GetInstancesGroupedByHandlers(s) - for handler, instances := range instancesByHandler { - if _, err := config.ValidateBuilder(handler, instances, s.Templates); err != nil { - insts := make([]string, 0) - for _, inst := range instances { - insts = append(insts, inst.Name) - } - errs = multierror.Append(errs, fmt.Errorf("error handler[%s]/instances[%s]: %v", - handler.Name, strings.Join(insts, ","), err)) - } - } - return errs -} diff --git a/mixer/pkg/runtime/config/validator/validator_test.go b/mixer/pkg/runtime/config/validator/validator_test.go deleted file mode 100644 index 140b84dd5c3f..000000000000 --- a/mixer/pkg/runtime/config/validator/validator_test.go +++ /dev/null @@ -1,344 +0,0 @@ -// Copyright 2017 Istio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package validator - -import ( - "context" - "errors" - "path/filepath" - "reflect" - "strings" - "testing" - - "github.com/gogo/protobuf/proto" - "github.com/gogo/protobuf/types" - multierror "github.com/hashicorp/go-multierror" - "k8s.io/apimachinery/pkg/runtime/schema" - - cpb "istio.io/api/policy/v1beta1" - adapter2 "istio.io/istio/mixer/adapter" - "istio.io/istio/mixer/pkg/adapter" - "istio.io/istio/mixer/pkg/config" - "istio.io/istio/mixer/pkg/config/crd" - "istio.io/istio/mixer/pkg/config/store" - "istio.io/istio/mixer/pkg/template" - template2 "istio.io/istio/mixer/template" -) - -// testAdapterConfig is a types.Struct instance which represents the -// adapter config used in the mixer/testdata/config/listentry.yaml. -// This is commonly used in the test cases and expectations in this file. -var testAdapterConfig = &types.Struct{ - Fields: map[string]*types.Value{ - "overrides": { - Kind: &types.Value_ListValue{ListValue: &types.ListValue{ - Values: []*types.Value{ - {Kind: &types.Value_StringValue{StringValue: "v1"}}, - {Kind: &types.Value_StringValue{StringValue: "v2"}}, - }, - }}, - }, - "blacklist": {Kind: &types.Value_BoolValue{BoolValue: false}}, - }, -} - -type dummyHandlerBldr struct { - want proto.Message - got proto.Message -} - -func (d *dummyHandlerBldr) SetAdapterConfig(cfg adapter.Config) { - d.got = cfg -} - -func (d *dummyHandlerBldr) Validate() *adapter.ConfigErrors { - var err *adapter.ConfigErrors - if !reflect.DeepEqual(d.want, d.got) { - err = err.Appendf("", "Got %v, Want %v", d.got, d.want) - } - return err -} - -func (d *dummyHandlerBldr) Build(ctx context.Context, env adapter.Env) (adapter.Handler, error) { - return nil, errors.New("dummy can't build") -} - -func getValidatorForTest() (*Validator, error) { - path, err := filepath.Abs("../../../../testdata/config") - if err != nil { - return nil, err - } - groupVersion := &schema.GroupVersion{Group: crd.ConfigAPIGroup, Version: crd.ConfigAPIVersion} - s, err := store.NewRegistry(config.StoreInventory()...).NewStore("fs://"+path, groupVersion, nil, []string{}) - if err != nil { - return nil, err - } - adapterInfo := make(map[string]*adapter.Info) - for _, y := range adapter2.Inventory() { - i := y() - adapterInfo[i.Name] = &i - } - - adapterInfo["listchecker"] = &adapter.Info{ - DefaultConfig: &types.Struct{}, - NewBuilder: func() adapter.HandlerBuilder { - return &dummyHandlerBldr{want: testAdapterConfig} - }, - SupportedTemplates: []string{ - "listentry", - }, - } - - templateInfo := make(map[string]*template.Info) - for x, y := range template2.SupportedTmplInfo { - tmp := y - templateInfo[x] = &tmp - } - - templateInfo["listentry"] = &template.Info{ - Name: "listentry", - CtrCfg: &types.Struct{}, - InferType: func(msg proto.Message, fn template.TypeEvalFn) (proto.Message, error) { - st := msg.(*types.Struct) - v, ok := st.Fields["value"] - if !ok { - return nil, errors.New("no value field") - } - value := v.GetStringValue() - if value == "" { - return nil, errors.New("not string value") - } - _, ierr := fn(value) - return nil, ierr - }, - BuilderSupportsTemplate: func(builder adapter.HandlerBuilder) bool { - return true - }, - } - - v, err := NewValidator(s, adapterInfo, templateInfo) - if err != nil { - return nil, err - } - return v.(*Validator), nil -} - -func updateEvent(keystr string, spec proto.Message) *store.Event { - keySegments := strings.Split(keystr, ".") - key := store.Key{Name: keySegments[0], Kind: keySegments[1], Namespace: keySegments[2]} - return &store.Event{Type: store.Update, Key: key, Value: &store.Resource{ - Metadata: store.ResourceMeta{Name: key.Name, Namespace: key.Namespace}, - Spec: spec, - }} -} - -func deleteEvent(keystr string) *store.Event { - keySegments := strings.Split(keystr, ".") - return &store.Event{Type: store.Delete, Key: store.Key{Name: keySegments[0], Kind: keySegments[1], Namespace: keySegments[2]}} -} - -func TestValidator(t *testing.T) { - for _, cc := range []struct { - title string - evs []*store.Event - ok bool - wantErr string - }{ - { - "new rule", - []*store.Event{updateEvent("test.rule.default", &cpb.Rule{ - Actions: []*cpb.Action{ - {Handler: "staticversion.handler.istio-system", Instances: []string{"appversion.listentry.istio-system"}}, - }})}, - true, - "", - }, - { - "update rule", - []*store.Event{updateEvent("checkwl.rule.istio-system", &cpb.Rule{ - Actions: []*cpb.Action{ - {Handler: "staticversion.handler", Instances: []string{"appversion.listentry"}}, - }})}, - true, - "", - }, - { - "delete rule", - []*store.Event{deleteEvent("checkwl.rule.istio-system")}, - true, - "", - }, - { - "invalid updating rule: match syntax error", - []*store.Event{updateEvent("test.rule.default", &cpb.Rule{Match: "foo"})}, - false, - "rule='test.rule.default'.Match: unknown attribute foo", - }, - { - "invalid updating rule: match type error", - []*store.Event{updateEvent("test.rule.default", &cpb.Rule{Match: "1"})}, - false, - "rule='test.rule.default'.Match: expression '1' evaluated to type INT64, expected type BOOL", - }, - { - "invalid updating rule: reference not found", - []*store.Event{updateEvent("test.rule.default", &cpb.Rule{Actions: []*cpb.Action{{Handler: "nonexistent.listchecker.istio-system"}}})}, - false, - "action='test.rule.default[0]': Handler not found: handler='nonexistent.listchecker.istio-system'", - }, - { - "adding adapter", - []*store.Event{updateEvent("test.listchecker.default", testAdapterConfig)}, - true, - "", - }, - { - "adding instance", - []*store.Event{updateEvent("test.listentry.default", &types.Struct{Fields: map[string]*types.Value{ - "value": {Kind: &types.Value_StringValue{StringValue: "0"}}, - }})}, - true, - "", - }, - { - "adapter validation failure", - []*store.Event{ - updateEvent("test.listchecker.default", &types.Struct{}), - updateEvent("testInst.listentry.default", &types.Struct{Fields: map[string]*types.Value{ - "value": {Kind: &types.Value_StringValue{StringValue: "0"}}, - }}), - updateEvent("r1.rule.default", &cpb.Rule{ - Actions: []*cpb.Action{ - {Handler: "test.listchecker.default", Instances: []string{"testInst.listentry.default"}}, - }}), - }, - false, - "handler[test.listchecker.default]/instances[testInst.listentry.default]: adapter validation failed", - }, - { - "invalid instance", - []*store.Event{updateEvent("test.listentry.default", &types.Struct{})}, - false, - "instance='test.listentry.default': no value field", - }, - { - "invalid instance syntax", - []*store.Event{updateEvent("test.listentry.default", &types.Struct{Fields: map[string]*types.Value{ - "value": {Kind: &types.Value_StringValue{StringValue: ""}}, - }})}, - false, - "instance='test.listentry.default': not string value", - }, - { - "invalid delete handler", - []*store.Event{deleteEvent("staticversion.handler.istio-system")}, - false, - "action='checkwl.rule.istio-system[0]': Handler not found: handler='staticversion'", - }, - { - "invalid delete instance", - []*store.Event{deleteEvent("appversion.listentry.istio-system")}, - false, - "action='checkwl.rule.istio-system[0]': Instance not found: instance='appversion.listentry'", - }, - { - "invalid removal of attributemanifest", - []*store.Event{deleteEvent("kubernetes.attributemanifest.istio-system")}, - false, - "", - }, - } { - t.Run(cc.title, func(tt *testing.T) { - v, err := getValidatorForTest() - if err != nil { - tt.Fatal(err) - } - defer v.Stop() - var result *multierror.Error - for _, ev := range cc.evs { - e := v.Validate(ev) - result = multierror.Append(result, e) - } - ok := result.ErrorOrNil() == nil - if cc.ok != ok { - tt.Errorf("Got %v, Want %v", result.ErrorOrNil(), cc.ok) - } - if cc.wantErr != "" { - if result.Error() == "" { - tt.Errorf("Got no error, Want err %s", cc.wantErr) - } - if !strings.Contains(result.Error(), cc.wantErr) { - tt.Logf("Got error %s, Want err %s", result.Error(), cc.wantErr) - } - } - }) - } -} - -func TestValidatorToRememberValidation(t *testing.T) { - t.Skip("https://github.com/istio/istio/issues/7696") - for _, c := range []struct { - title string - ev1 *store.Event - evs []*store.Event - }{ - { - "reference", - updateEvent("test.rule.default", &cpb.Rule{ - Actions: []*cpb.Action{ - {Handler: "test.listchecker", Instances: []string{"appversion.listentry.istio-system"}}, - }, - }), - []*store.Event{updateEvent("test.listchecker.default", testAdapterConfig)}, - }, - { - "deletion order", - deleteEvent("checkwl.rule.istio-system"), - []*store.Event{ - deleteEvent("staticversion.listchecker.istio-system"), - deleteEvent("appversion.listentry.istio-system"), - }, - }, - { - "deleting attribute manifests", - deleteEvent("kubernetes.attributemanifest.istio-system"), - []*store.Event{ - deleteEvent("staticversion.listchecker.istio-system"), - deleteEvent("appversion.listentry.istio-system"), - deleteEvent("checkwl.rule.istio-system"), - }, - }, - } { - t.Run(c.title, func(tt *testing.T) { - v, err := getValidatorForTest() - if err != nil { - tt.Fatal(err) - } - defer v.Stop() - - if err = v.Validate(c.ev1); err == nil { - tt.Error("Got nil, Want error") - } - for _, ev := range c.evs { - if err = v.Validate(ev); err != nil { - tt.Errorf("Got %v, Want nil", err) - } - } - if err = v.Validate(c.ev1); err != nil { - tt.Errorf("Got %v, Want nil", err) - } - }) - } -} diff --git a/mixer/pkg/runtime/runtime.go b/mixer/pkg/runtime/runtime.go index a028eda9ee51..cb80668d0f82 100644 --- a/mixer/pkg/runtime/runtime.go +++ b/mixer/pkg/runtime/runtime.go @@ -148,7 +148,11 @@ func (c *Runtime) onConfigChange(events []*store.Event) { } func (c *Runtime) processNewConfig() { - newSnapshot, _ := c.ephemeral.BuildSnapshot() + newSnapshot, err := c.ephemeral.BuildSnapshot() + log.Infof("Built new config.Snapshot: id='%d'", newSnapshot.ID) + if err != nil { + log.Error(err.Error()) + } oldHandlers := c.handlers diff --git a/mixer/pkg/runtime/testing/data/data.go b/mixer/pkg/runtime/testing/data/data.go index 593f58a88e27..dbcee53886d4 100644 --- a/mixer/pkg/runtime/testing/data/data.go +++ b/mixer/pkg/runtime/testing/data/data.go @@ -297,7 +297,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - selector: 'true' + match: 'true' actions: - handler: hcheck1.acheck instances: @@ -312,7 +312,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - selector: needmorecheese + match: needmorecheese actions: - handler: hcheck1.acheck instances: @@ -410,7 +410,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - selector: destination.name + match: destination.name actions: - handler: hcheck1.acheck instances: diff --git a/mixer/pkg/validate/validator.go b/mixer/pkg/validate/validator.go new file mode 100644 index 000000000000..d6bbf7bde764 --- /dev/null +++ b/mixer/pkg/validate/validator.go @@ -0,0 +1,87 @@ +// Copyright 2017 Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package validate + +import ( + "github.com/gogo/protobuf/proto" + + "istio.io/istio/mixer/adapter/metadata" + "istio.io/istio/mixer/pkg/adapter" + "istio.io/istio/mixer/pkg/config/store" + runtimeConfig "istio.io/istio/mixer/pkg/runtime/config" + "istio.io/istio/mixer/pkg/template" + generatedTmplRepo "istio.io/istio/mixer/template" + "istio.io/istio/pkg/log" +) + +// validator provides the default structural validation +type validator struct { + kinds map[string]proto.Message + config *runtimeConfig.Ephemeral +} + +// NewValidator creates a default validator which validates the structure through registered +// kinds and optionally referential integrity and expressions if stateful verification is enabled. +func NewValidator(adapters map[string]*adapter.Info, templates map[string]*template.Info, stateful bool) store.BackendValidator { + out := &validator{ + kinds: runtimeConfig.KindMap(adapters, templates), + } + if stateful { + out.config = runtimeConfig.NewEphemeral(templates, adapters) + } + return out +} + +// NewDefaultValidator creates a validator using compiled templates and adapter descriptors. +// It does not depend on actual handler/builder interfaces from the compiled-in adapters. +func NewDefaultValidator(stateful bool) store.BackendValidator { + info := generatedTmplRepo.SupportedTmplInfo + templates := make(map[string]*template.Info, len(info)) + for k := range info { + t := info[k] + templates[k] = &t + } + adapters := metadata.InfoMap() + return NewValidator(adapters, templates, stateful) +} + +func (v *validator) Validate(bev *store.BackendEvent) error { + _, ok := v.kinds[bev.Key.Kind] + if !ok { + // Pass unrecognized kinds -- they should be validated by somewhere else. + log.Debugf("unrecognized kind %s is requested to validate", bev.Key.Kind) + return nil + } + + ev, err := store.ConvertValue(*bev, v.kinds) + if err != nil { + return err + } + + // optional deep validation + if v.config != nil { + v.config.ApplyEvent([]*store.Event{&ev}) + if _, err = v.config.BuildSnapshot(); err != nil { + return err + } + } + + return nil +} + +func (v *validator) SupportsKind(kind string) bool { + _, exists := v.kinds[kind] + return exists +} diff --git a/mixer/pkg/validate/validator_test.go b/mixer/pkg/validate/validator_test.go new file mode 100644 index 000000000000..57c8e25d79f7 --- /dev/null +++ b/mixer/pkg/validate/validator_test.go @@ -0,0 +1,90 @@ +// Copyright 2017 Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package validate + +import ( + "errors" + "strings" + "testing" + + "istio.io/istio/mixer/pkg/config/store" +) + +func backendEvent(t store.ChangeType, kind, name string, spec map[string]interface{}) *store.BackendEvent { + namespace := "ns" + return &store.BackendEvent{ + Type: t, + Key: store.Key{Kind: kind, Namespace: namespace, Name: name}, + Value: &store.BackEndResource{ + Metadata: store.ResourceMeta{ + Name: name, + Namespace: namespace, + }, + Spec: spec, + }, + } +} + +func TestValidate(t *testing.T) { + for _, c := range []struct { + title string + ev *store.BackendEvent + want error + }{ + { + "update", + backendEvent(store.Update, "handler", "foo", map[string]interface{}{"compiledAdapter": "noop", "name": "default"}), + nil, + }, + { + "update error (shallow)", + backendEvent(store.Update, "handler", "foo", map[string]interface{}{"adapterr": "noop"}), + errors.New(`unknown field "adapterr" in v1beta1.Handler`), + }, + { + "update error (referential)", + backendEvent(store.Update, "handler", "foo", map[string]interface{}{"adapter": "prometheus"}), + errors.New(`handler='foo.handler.ns'.adapter: adapter 'prometheus' not found`), + }, + { + "delete", + backendEvent(store.Delete, "handler", "foo", nil), + nil, + }, + { + "unknown kinds", + backendEvent(store.Update, "unknown", "bar", map[string]interface{}{"foo": "bar"}), + nil, + }, + } { + t.Run(c.title, func(tt *testing.T) { + v := NewDefaultValidator(true) + err := v.Validate(c.ev) + if c.want == nil && err != nil { + tt.Errorf("Got %v, Want nil", err) + } else if c.want != nil && !strings.Contains(err.Error(), c.want.Error()) { + tt.Errorf("Got %v, Want to contain %s", err, c.want.Error()) + } + }) + } +} + +func TestSupportsKind(t *testing.T) { + v := NewDefaultValidator(true) + + if got := v.SupportsKind("instance"); !got { + t.Errorf("SupportsKind => got false for instance") + } +} diff --git a/mixer/test/e2e/e2e_report_test.go b/mixer/test/e2e/e2e_report_test.go index 2e533d5ed2e5..b3d4e7027714 100644 --- a/mixer/test/e2e/e2e_report_test.go +++ b/mixer/test/e2e/e2e_report_test.go @@ -88,7 +88,7 @@ metadata: name: rule1 namespace: istio-system spec: - selector: match(target.name, "*") + match: match(target.name, "*") actions: - handler: fakeHandlerConfig.handler instances: @@ -165,7 +165,7 @@ metadata: name: rule1 namespace: istio-system spec: - selector: match(target.name, "*") + match: match(target.name, "*") actions: - handler: fakeHandlerConfig.fakeHandler instances: @@ -238,7 +238,7 @@ metadata: name: rule1 namespace: istio-system spec: - selector: match(target.name, "some unknown thing") + match: match(target.name, "some unknown thing") actions: - handler: fakeHandlerConfig.fakeHandler instances: