diff --git a/cmd/ela-webhook/BUILD.bazel b/cmd/ela-webhook/BUILD.bazel index 3fd1cf3dd2dc..266d91d943e6 100644 --- a/cmd/ela-webhook/BUILD.bazel +++ b/cmd/ela-webhook/BUILD.bazel @@ -6,9 +6,11 @@ go_library( importpath = "github.com/elafros/elafros/cmd/ela-webhook", visibility = ["//visibility:private"], deps = [ + "//pkg/logging:go_default_library", "//pkg/signals:go_default_library", "//pkg/webhook:go_default_library", - "//vendor/github.com/golang/glog:go_default_library", + "//vendor/github.com/josephburnett/k8sflag/pkg/k8sflag:go_default_library", + "//vendor/go.uber.org/zap:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", ], diff --git a/cmd/ela-webhook/main.go b/cmd/ela-webhook/main.go index 26723e652708..d1fd949511f2 100644 --- a/cmd/ela-webhook/main.go +++ b/cmd/ela-webhook/main.go @@ -18,10 +18,12 @@ package main import ( "flag" + "go.uber.org/zap" + + "github.com/elafros/elafros/pkg/logging" "github.com/elafros/elafros/pkg/signals" "github.com/elafros/elafros/pkg/webhook" - - "github.com/golang/glog" + "github.com/josephburnett/k8sflag/pkg/k8sflag" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -29,19 +31,23 @@ import ( func main() { flag.Parse() - glog.Info("Starting the Configuration Webhook...") + loggingZapCfg := k8sflag.String("logging.zap-config", "") + logger := logging.NewLogger(loggingZapCfg.Get()).Named("ela-webhook") + defer logger.Sync() + + logger.Info("Starting the Configuration Webhook") // set up signals so we handle the first shutdown signal gracefully stopCh := signals.SetupSignalHandler() clusterConfig, err := rest.InClusterConfig() if err != nil { - glog.Fatal(err.Error()) + logger.Fatal("Failed to get in cluster config", zap.Error(err)) } clientset, err := kubernetes.NewForConfig(clusterConfig) if err != nil { - glog.Fatal(err) + logger.Fatal("Failed to get the client set", zap.Error(err)) } options := webhook.ControllerOptions{ @@ -51,9 +57,9 @@ func main() { SecretName: "ela-webhook-certs", WebhookName: "webhook.elafros.dev", } - controller, err := webhook.NewAdmissionController(clientset, options) + controller, err := webhook.NewAdmissionController(clientset, options, logger) if err != nil { - glog.Fatal(err) + logger.Fatal("Failed to create the admission controller", zap.Error(err)) } controller.Run(stopCh) } diff --git a/config/BUILD.bazel b/config/BUILD.bazel index b5877f9db955..66e279dea4d5 100644 --- a/config/BUILD.bazel +++ b/config/BUILD.bazel @@ -7,6 +7,11 @@ k8s_object( template = "elaconfig.yaml", ) +k8s_object( + name = "elawebhookconfig", + template = "elawebhookconfig.yaml", +) + k8s_object( name = "controller", images = { @@ -110,6 +115,7 @@ k8s_objects( ":authz", ":crds", ":elaconfig", + ":elawebhookconfig", ":controller", ":controllerservice", ":webhook", diff --git a/config/controller.yaml b/config/controller.yaml index 0271cc22094f..a3aadb52a09c 100644 --- a/config/controller.yaml +++ b/config/controller.yaml @@ -31,8 +31,6 @@ spec: # and substituted here. image: github.com/elafros/elafros/cmd/ela-controller args: - - "-logtostderr=true" - - "-stderrthreshold=INFO" - "-queueSidecarImage" # This is the Go import path for the binary that is containerized # and substituted here. diff --git a/config/elawebhookconfig.yaml b/config/elawebhookconfig.yaml new file mode 100644 index 000000000000..1bbad46e0363 --- /dev/null +++ b/config/elawebhookconfig.yaml @@ -0,0 +1,46 @@ +# Copyright 2018 Google LLC +# +# 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 +# +# https://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. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: ela-webhook-config + namespace: ela-system +data: + # Logging configuration + logging.zap-config: | + { + "level": "info", + "development": false, + "sampling": { + "initial": 100, + "thereafter": 100 + }, + "outputPaths": ["stdout"], + "errorOutputPaths": ["stderr"], + "encoding": "json", + "encoderConfig": { + "timeKey": "", + "levelKey": "level", + "nameKey": "logger", + "callerKey": "caller", + "messageKey": "msg", + "stacktraceKey": "stacktrace", + "lineEnding": "", + "levelEncoder": "", + "timeEncoder": "", + "durationEncoder": "", + "callerEncoder": "" + } + } diff --git a/config/webhook.yaml b/config/webhook.yaml index 8eb1ac482506..5cdceeafa8e9 100644 --- a/config/webhook.yaml +++ b/config/webhook.yaml @@ -31,6 +31,10 @@ spec: # This is the Go import path for the binary that is containerized # and substituted here. image: github.com/elafros/elafros/cmd/ela-webhook - args: - - "-logtostderr=true" - - "-stderrthreshold=INFO" + volumeMounts: + - name: ela-webhook-config + mountPath: /etc/config + volumes: + - name: ela-webhook-config + configMap: + name: ela-webhook-config diff --git a/pkg/logging/logkey/constants.go b/pkg/logging/logkey/constants.go index d560aacbdca7..81bbe74acea1 100644 --- a/pkg/logging/logkey/constants.go +++ b/pkg/logging/logkey/constants.go @@ -40,4 +40,22 @@ const ( // JSONConfig is the key used for JSON configurations (not to be confused by the Configuration object) JSONConfig = "elafros.dev/jsonconfig" + + // Kind is the key used to represent kind of an object in logs + Kind = "elafros.dev/kind" + + // Name is the key used to represent name of an object in logs + Name = "elafros.dev/name" + + // Operation is the key used to represent an operation in logs + Operation = "elafros.dev/operation" + + // Resource is the key used to represent a resource in logs + Resource = "elafros.dev/resource" + + // SubResource is a generic key used to represent a sub-resource in logs + SubResource = "elafros.dev/subresource" + + // UserInfo is the key used to represent a user information in logs + UserInfo = "elafros.dev/userinfo" ) diff --git a/pkg/webhook/BUILD.bazel b/pkg/webhook/BUILD.bazel index dfee72233fd3..fcf4032022f8 100644 --- a/pkg/webhook/BUILD.bazel +++ b/pkg/webhook/BUILD.bazel @@ -15,9 +15,11 @@ go_library( deps = [ "//pkg/apis/ela:go_default_library", "//pkg/apis/ela/v1alpha1:go_default_library", - "//vendor/github.com/golang/glog:go_default_library", + "//pkg/logging:go_default_library", + "//pkg/logging/logkey:go_default_library", "//vendor/github.com/google/go-cmp/cmp:go_default_library", "//vendor/github.com/mattbaird/jsonpatch:go_default_library", + "//vendor/go.uber.org/zap:go_default_library", "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", @@ -41,7 +43,9 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/ela/v1alpha1:go_default_library", + "//pkg/logging:go_default_library", "//vendor/github.com/mattbaird/jsonpatch:go_default_library", + "//vendor/go.uber.org/zap:go_default_library", "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/webhook/certs.go b/pkg/webhook/certs.go index 7a611593a57f..4c3730a15334 100644 --- a/pkg/webhook/certs.go +++ b/pkg/webhook/certs.go @@ -16,6 +16,7 @@ limitations under the License. package webhook import ( + "context" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -25,7 +26,9 @@ import ( "math/big" "time" - "github.com/golang/glog" + "go.uber.org/zap" + + "github.com/elafros/elafros/pkg/logging" ) const ( @@ -98,22 +101,23 @@ func createCert(template, parent *x509.Certificate, pub interface{}, parentPriv return } -func createCA() (*rsa.PrivateKey, *x509.Certificate, []byte, error) { +func createCA(ctx context.Context) (*rsa.PrivateKey, *x509.Certificate, []byte, error) { + logger := logging.FromContext(ctx) rootKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { - glog.Warningf("error generating random key: %s", err) + logger.Error("error generating random key", zap.Error(err)) return nil, nil, nil, err } rootCertTmpl, err := createCACertTemplate() if err != nil { - glog.Warningf("error generating CA cert: %s", err) + logger.Error("error generating CA cert", zap.Error(err)) return nil, nil, nil, err } rootCert, rootCertPEM, err := createCert(rootCertTmpl, rootCertTmpl, &rootKey.PublicKey, rootKey) if err != nil { - glog.Warningf("error signing the CA cert: %s", err) + logger.Error("error signing the CA cert", zap.Error(err)) return nil, nil, nil, err } return rootKey, rootCert, rootCertPEM, nil @@ -123,9 +127,10 @@ func createCA() (*rsa.PrivateKey, *x509.Certificate, []byte, error) { // key for the server. serverKey and serverCert are used by the server // to establish trust for clients, CA certificate is used by the // client to verify the server authentication chain. -func CreateCerts() (serverKey, serverCert, caCert []byte, err error) { +func CreateCerts(ctx context.Context) (serverKey, serverCert, caCert []byte, err error) { + logger := logging.FromContext(ctx) // First create a CA certificate and private key - caKey, caCertificate, caCertificatePEM, err := createCA() + caKey, caCertificate, caCertificatePEM, err := createCA(ctx) if err != nil { return nil, nil, nil, err } @@ -133,19 +138,19 @@ func CreateCerts() (serverKey, serverCert, caCert []byte, err error) { // Then create the private key for the serving cert servKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { - glog.Warningf("error generating random key: %s", err) + logger.Error("error generating random key", zap.Error(err)) return nil, nil, nil, err } servCertTemplate, err := createServerCertTemplate() if err != nil { - glog.Warningf("failed to create the server certificate template: %s", err) + logger.Error("failed to create the server certificate template", zap.Error(err)) return nil, nil, nil, err } // create a certificate which wraps the server's public key, sign it with the CA private key _, servCertPEM, err := createCert(servCertTemplate, caCertificate, &servKey.PublicKey, caKey) if err != nil { - glog.Warningf("error signing server certificate template: %s", err) + logger.Error("error signing server certificate template", zap.Error(err)) return nil, nil, nil, err } servKeyPEM := pem.EncodeToMemory(&pem.Block{ diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 7a7cfa22ea53..032b761c1525 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -16,6 +16,7 @@ limitations under the License. package webhook import ( + "context" "errors" "fmt" "path" @@ -23,7 +24,7 @@ import ( "strings" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" - "github.com/golang/glog" + "github.com/elafros/elafros/pkg/logging" "github.com/mattbaird/jsonpatch" corev1 "k8s.io/api/core/v1" ) @@ -40,19 +41,21 @@ var ( errEmptySpecInConfiguration = errMissingField("spec") errEmptyRevisionTemplateInSpec = errMissingField("spec.revisionTemplate") errEmptyContainerInRevisionTemplate = errMissingField("spec.revisionTemplate.spec.container") - errInvalidConfigurationInput = errors.New("Failed to convert input into Configuration.") + errInvalidConfigurationInput = errors.New("failed to convert input into Configuration") ) // ValidateConfiguration is Configuration resource specific validation and mutation handler -func ValidateConfiguration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { - _, newConfiguration, err := unmarshalConfigurations(old, new, "ValidateConfiguration") - if err != nil { - return err - } - if err := validateConfiguration(newConfiguration); err != nil { - return err +func ValidateConfiguration(ctx context.Context) ResourceCallback { + return func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { + _, newConfiguration, err := unmarshalConfigurations(ctx, old, new, "ValidateConfiguration") + if err != nil { + return err + } + if err := validateConfiguration(newConfiguration); err != nil { + return err + } + return nil } - return nil } func validateConfiguration(configuration *v1alpha1.Configuration) error { @@ -122,16 +125,19 @@ func validateContainer(container corev1.Container) error { return nil } -func SetConfigurationDefaults(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { - _, config, err := unmarshalConfigurations(nil, crd, "SetConfigurationDefaults") - if err != nil { - return err - } +// SetConfigurationDefaults set defaults on an configurations. +func SetConfigurationDefaults(ctx context.Context) ResourceDefaulter { + return func(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { + _, config, err := unmarshalConfigurations(ctx, nil, crd, "SetConfigurationDefaults") + if err != nil { + return err + } - return SetConfigurationSpecDefaults(patches, "/spec", config.Spec) + return setConfigurationSpecDefaults(patches, "/spec", config.Spec) + } } -func SetConfigurationSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patchBase string, spec v1alpha1.ConfigurationSpec) error { +func setConfigurationSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patchBase string, spec v1alpha1.ConfigurationSpec) error { if spec.RevisionTemplate.Spec.ConcurrencyModel == "" { *patches = append(*patches, jsonpatch.JsonPatchOperation{ Operation: "add", @@ -142,7 +148,9 @@ func SetConfigurationSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patch return nil } -func unmarshalConfigurations(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Configuration, *v1alpha1.Configuration, error) { +func unmarshalConfigurations( + ctx context.Context, old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Configuration, *v1alpha1.Configuration, error) { + logger := logging.FromContext(ctx) var oldConfiguration *v1alpha1.Configuration if old != nil { var ok bool @@ -151,13 +159,13 @@ func unmarshalConfigurations(old GenericCRD, new GenericCRD, fnName string) (*v1 return nil, nil, errInvalidConfigurationInput } } - glog.Infof("%s: OLD Configuration is\n%+v", fnName, oldConfiguration) + logger.Infof("%s: OLD Configuration is\n%+v", fnName, oldConfiguration) newConfiguration, ok := new.(*v1alpha1.Configuration) if !ok { return nil, nil, errInvalidConfigurationInput } - glog.Infof("%s: NEW Configuration is\n%+v", fnName, newConfiguration) + logger.Infof("%s: NEW Configuration is\n%+v", fnName, newConfiguration) return oldConfiguration, newConfiguration, nil } diff --git a/pkg/webhook/configuration_test.go b/pkg/webhook/configuration_test.go index 18aea9c2a708..d0c27280f48c 100644 --- a/pkg/webhook/configuration_test.go +++ b/pkg/webhook/configuration_test.go @@ -29,13 +29,13 @@ import ( func TestValidConfigurationAllowed(t *testing.T) { configuration := createConfiguration(testGeneration, testConfigurationName) - if err := ValidateConfiguration(nil, &configuration, &configuration); err != nil { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err != nil { t.Fatalf("Expected allowed. Failed with %s", err) } } func TestEmptyConfigurationNotAllowed(t *testing.T) { - if err := ValidateConfiguration(nil, nil, nil); err != errInvalidConfigurationInput { + if err := ValidateConfiguration(testCtx)(nil, nil, nil); err != errInvalidConfigurationInput { t.Fatalf("Expected: %s. Failed with %s", errInvalidConfigurationInput, err) } } @@ -49,7 +49,7 @@ func TestEmptySpecInConfigurationNotAllowed(t *testing.T) { Spec: v1alpha1.ConfigurationSpec{}, } - if err := ValidateConfiguration(nil, &configuration, &configuration); err != errEmptySpecInConfiguration { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err != errEmptySpecInConfiguration { t.Fatalf("Expected: %s. Failed with %s", errEmptySpecInConfiguration, err) } } @@ -66,7 +66,7 @@ func TestEmptyTemplateInSpecNotAllowed(t *testing.T) { }, } - if err := ValidateConfiguration(nil, &configuration, &configuration); err != errEmptyRevisionTemplateInSpec { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err != errEmptyRevisionTemplateInSpec { t.Fatalf("Expected: %s. Failed with %s", errEmptyRevisionTemplateInSpec, err) } } @@ -87,7 +87,7 @@ func TestEmptyContainerNotAllowed(t *testing.T) { }, } - if err := ValidateConfiguration(nil, &configuration, &configuration); err != errEmptyContainerInRevisionTemplate { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err != errEmptyContainerInRevisionTemplate { t.Fatalf("Expected: %v. Failed with %v", errEmptyRevisionTemplateInSpec, err) } } @@ -112,7 +112,7 @@ func TestServingStateNotAllowed(t *testing.T) { }, } expected := fmt.Sprintf("The configuration spec must not set the field(s): revisionTemplate.spec.servingState") - if err := ValidateConfiguration(nil, &configuration, &configuration); err == nil || err.Error() != expected { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err == nil || err.Error() != expected { t.Fatalf("Result of ValidateConfiguration function: %s. Expected: %s.", err, expected) } } @@ -155,17 +155,17 @@ func TestUnwantedFieldInContainerNotAllowed(t *testing.T) { "revisionTemplate.spec.container.volumeMounts", } expected := fmt.Sprintf("The configuration spec must not set the field(s): %s", strings.Join(unwanted, ", ")) - if err := ValidateConfiguration(nil, &configuration, &configuration); err == nil || err.Error() != expected { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err == nil || err.Error() != expected { t.Fatalf("Expected: %s. Failed with %s", expected, err) } configuration.Spec.RevisionTemplate.Spec.Container.Name = "" expected = fmt.Sprintf("The configuration spec must not set the field(s): %s", strings.Join(unwanted[1:], ", ")) - if err := ValidateConfiguration(nil, &configuration, &configuration); err == nil || err.Error() != expected { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err == nil || err.Error() != expected { t.Fatalf("Expected: %s. Failed with %s", expected, err) } configuration.Spec.RevisionTemplate.Spec.Container.Resources = corev1.ResourceRequirements{} expected = fmt.Sprintf("The configuration spec must not set the field(s): %s", strings.Join(unwanted[2:], ", ")) - if err := ValidateConfiguration(nil, &configuration, &configuration); err == nil || err.Error() != expected { + if err := ValidateConfiguration(testCtx)(nil, &configuration, &configuration); err == nil || err.Error() != expected { t.Fatalf("Expected: %s. Failed with %s", expected, err) } } diff --git a/pkg/webhook/revision.go b/pkg/webhook/revision.go index 861db0e572bf..4984e951bc71 100644 --- a/pkg/webhook/revision.go +++ b/pkg/webhook/revision.go @@ -16,51 +16,57 @@ limitations under the License. package webhook import ( + "context" "errors" "fmt" "path" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" - "github.com/golang/glog" + "github.com/elafros/elafros/pkg/logging" "github.com/google/go-cmp/cmp" "github.com/mattbaird/jsonpatch" ) var ( - errInvalidRevisionInput = errors.New("Failed to convert input into Revision.") + errInvalidRevisionInput = errors.New("failed to convert input into Revision") ) // ValidateRevision is Revision resource specific validation and mutation handler -func ValidateRevision(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { - o, n, err := unmarshalRevisions(old, new, "ValidateRevision") - if err != nil { - return err - } +func ValidateRevision(ctx context.Context) ResourceCallback { + return func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { + o, n, err := unmarshalRevisions(ctx, old, new, "ValidateRevision") + if err != nil { + return err + } - // When we have an "old" object, check for changes. - if o != nil { - // The autoscaler is allowed to change these fields, so clear them. - o.Spec.ServingState = "" - n.Spec.ServingState = "" + // When we have an "old" object, check for changes. + if o != nil { + // The autoscaler is allowed to change these fields, so clear them. + o.Spec.ServingState = "" + n.Spec.ServingState = "" - if diff := cmp.Diff(o.Spec, n.Spec); diff != "" { - return fmt.Errorf("Revision spec should not change (-old +new): %s", diff) + if diff := cmp.Diff(o.Spec, n.Spec); diff != "" { + return fmt.Errorf("Revision spec should not change (-old +new): %s", diff) + } } - } - return nil + return nil + } } -func SetRevisionDefaults(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { - _, revision, err := unmarshalRevisions(nil, crd, "SetRevisionDefaults") - if err != nil { - return err - } +// SetRevisionDefaults set defaults on an revisions. +func SetRevisionDefaults(ctx context.Context) ResourceDefaulter { + return func(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { + _, revision, err := unmarshalRevisions(ctx, nil, crd, "SetRevisionDefaults") + if err != nil { + return err + } - return SetRevisionSpecDefaults(patches, "/spec", revision.Spec) + return setRevisionSpecDefaults(patches, "/spec", revision.Spec) + } } -func SetRevisionSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patchBase string, spec v1alpha1.RevisionSpec) error { +func setRevisionSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patchBase string, spec v1alpha1.RevisionSpec) error { if spec.ServingState == "" { *patches = append(*patches, jsonpatch.JsonPatchOperation{ Operation: "add", @@ -80,7 +86,8 @@ func SetRevisionSpecDefaults(patches *[]jsonpatch.JsonPatchOperation, patchBase return nil } -func unmarshalRevisions(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Revision, *v1alpha1.Revision, error) { +func unmarshalRevisions(ctx context.Context, old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Revision, *v1alpha1.Revision, error) { + logger := logging.FromContext(ctx) var oldRevision *v1alpha1.Revision if old != nil { var ok bool @@ -89,13 +96,13 @@ func unmarshalRevisions(old GenericCRD, new GenericCRD, fnName string) (*v1alpha return nil, nil, errInvalidRevisionInput } } - glog.Infof("%s: OLD Revision is\n%+v", fnName, oldRevision) + logger.Infof("%s: OLD Revision is\n%+v", fnName, oldRevision) newRevision, ok := new.(*v1alpha1.Revision) if !ok { return nil, nil, errInvalidRevisionInput } - glog.Infof("%s: NEW Revision is\n%+v", fnName, newRevision) + logger.Infof("%s: NEW Revision is\n%+v", fnName, newRevision) return oldRevision, newRevision, nil } diff --git a/pkg/webhook/route.go b/pkg/webhook/route.go index 0499e78e480f..d31a8eee29fe 100644 --- a/pkg/webhook/route.go +++ b/pkg/webhook/route.go @@ -16,35 +16,38 @@ limitations under the License. package webhook import ( + "context" "errors" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" - "github.com/golang/glog" + "github.com/elafros/elafros/pkg/logging" "github.com/mattbaird/jsonpatch" ) var ( - errInvalidRevisions = errors.New("The route must have exactly one of revisionName or configurationName in traffic field.") - errInvalidRouteInput = errors.New("Failed to convert input into Route.") - errInvalidTargetPercentSum = errors.New("The route must have traffic percent sum equal to 100.") - errNegativeTargetPercent = errors.New("The route cannot have a negative traffic percent.") - errTrafficTargetsNotUnique = errors.New("The traffic targets must be unique.") + errInvalidRevisions = errors.New("the route must have exactly one of revisionName or configurationName in traffic field") + errInvalidRouteInput = errors.New("failed to convert input into Route") + errInvalidTargetPercentSum = errors.New("the route must have traffic percent sum equal to 100") + errNegativeTargetPercent = errors.New("the route cannot have a negative traffic percent") + errTrafficTargetsNotUnique = errors.New("the traffic targets must be unique") ) // ValidateRoute is Route resource specific validation and mutation handler -func ValidateRoute(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { - _, newRoute, err := unmarshalRoutes(old, new, "ValidateRoute") - if err != nil { - return err - } - if err := validateTrafficTarget(newRoute); err != nil { - return err - } - if err := validateUniqueTrafficTarget(newRoute); err != nil { - return err - } +func ValidateRoute(ctx context.Context) ResourceCallback { + return func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { + _, newRoute, err := unmarshalRoutes(ctx, old, new, "ValidateRoute") + if err != nil { + return err + } + if err := validateTrafficTarget(newRoute); err != nil { + return err + } + if err := validateUniqueTrafficTarget(newRoute); err != nil { + return err + } - return nil + return nil + } } func validateTrafficTarget(route *v1alpha1.Route) error { @@ -103,7 +106,9 @@ func validateUniqueTrafficTarget(route *v1alpha1.Route) error { return nil } -func unmarshalRoutes(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Route, *v1alpha1.Route, error) { +func unmarshalRoutes( + ctx context.Context, old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Route, *v1alpha1.Route, error) { + logger := logging.FromContext(ctx) var oldRoute *v1alpha1.Route if old != nil { var ok bool @@ -112,13 +117,13 @@ func unmarshalRoutes(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.R return nil, nil, errInvalidRouteInput } } - glog.Infof("%s: OLD Route is\n%+v", fnName, oldRoute) + logger.Infof("%s: OLD Route is\n%+v", fnName, oldRoute) newRoute, ok := new.(*v1alpha1.Route) if !ok { return nil, nil, errInvalidRouteInput } - glog.Infof("%s: NEW Route is\n%+v", fnName, newRoute) + logger.Infof("%s: NEW Route is\n%+v", fnName, newRoute) return oldRoute, newRoute, nil } diff --git a/pkg/webhook/route_test.go b/pkg/webhook/route_test.go index b0fdbdf94fb7..abd114285a9f 100644 --- a/pkg/webhook/route_test.go +++ b/pkg/webhook/route_test.go @@ -48,7 +48,7 @@ func TestValidRouteWithTrafficAllowed(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != nil { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != nil { t.Fatalf("Expected allowed, but failed with: %s.", err) } } @@ -56,7 +56,7 @@ func TestValidRouteWithTrafficAllowed(t *testing.T) { func TestEmptyTrafficTargetWithoutTrafficAllowed(t *testing.T) { route := createRouteWithTraffic(nil) - if err := ValidateRoute(nil, &route, &route); err != nil { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != nil { t.Fatalf("Expected allowed, but failed with: %s.", err) } } @@ -69,7 +69,7 @@ func TestNoneRouteTypeForOldResourceNotAllowed(t *testing.T) { }, } - if err := ValidateRoute(nil, &revision, &revision); err != errInvalidRouteInput { + if err := ValidateRoute(testCtx)(nil, &revision, &revision); err != errInvalidRouteInput { t.Fatalf( "Expected: %s. Failed with: %s.", errInvalidRouteInput, err) } @@ -83,7 +83,7 @@ func TestNoneRouteTypeForNewResourceNotAllowed(t *testing.T) { }, } - if err := ValidateRoute(nil, nil, &revision); err != errInvalidRouteInput { + if err := ValidateRoute(testCtx)(nil, nil, &revision); err != errInvalidRouteInput { t.Fatalf( "Expected: %s. Failed with: %s.", errInvalidRouteInput, err) } @@ -97,7 +97,7 @@ func TestEmptyRevisionAndConfigurationInOneTargetNotAllowed(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != errInvalidRevisions { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != errInvalidRevisions { t.Fatalf( "Expected: %s. Failed with: %s.", errInvalidRevisions, err) } @@ -113,7 +113,7 @@ func TestBothRevisionAndConfigurationInOneTargetNotAllowed(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != errInvalidRevisions { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != errInvalidRevisions { t.Fatalf( "Expected: %s. Failed with: %s.", errInvalidRevisions, err) } @@ -128,7 +128,7 @@ func TestNegativeTargetPercentNotAllowed(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != errNegativeTargetPercent { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != errNegativeTargetPercent { t.Fatalf( "Expected: %s. Failed with: %s.", errNegativeTargetPercent, err) } @@ -146,7 +146,7 @@ func TestNotAllowedIfTrafficPercentSumIsNot100(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != errInvalidTargetPercentSum { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != errInvalidTargetPercentSum { t.Fatalf( "Expected: %s. Failed with: %s.", errInvalidTargetPercentSum, err) } @@ -167,7 +167,7 @@ func TestNotAllowedIfTrafficNamesNotUnique(t *testing.T) { }, }) - if err := ValidateRoute(nil, &route, &route); err != errTrafficTargetsNotUnique { + if err := ValidateRoute(testCtx)(nil, &route, &route); err != errTrafficTargetsNotUnique { t.Fatalf( "Expected: %s. Failed with: %s.", errTrafficTargetsNotUnique, err) } diff --git a/pkg/webhook/service.go b/pkg/webhook/service.go index 5a8aa7890818..ed4178d4a587 100644 --- a/pkg/webhook/service.go +++ b/pkg/webhook/service.go @@ -16,19 +16,20 @@ limitations under the License. package webhook import ( + "context" "errors" "fmt" "reflect" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" - "github.com/golang/glog" + "github.com/elafros/elafros/pkg/logging" "github.com/mattbaird/jsonpatch" ) var ( - errInvalidRollouts = errors.New("The service must have exactly one of runLatest or pinned in spec field.") - errMissingRevisionName = errors.New("The PinnedType must have revision specified.") - errInvalidServiceInput = errors.New("Failed to convert input into Service.") + errInvalidRollouts = errors.New("the service must have exactly one of runLatest or pinned in spec field") + errMissingRevisionName = errors.New("the PinnedType must have revision specified") + errInvalidServiceInput = errors.New("failed to convert input into Service") ) func errServiceMissingField(fieldPath string) error { @@ -40,14 +41,16 @@ func errServiceDisallowedFields(fieldPaths string) error { } // ValidateService is Service resource specific validation and mutation handler -func ValidateService(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { - // We only care about the new one, old one gets flagged as an error in unmarshal. - _, newService, err := unmarshalServices(old, new, "ValidateService") - if err != nil { - return err - } +func ValidateService(ctx context.Context) ResourceCallback { + return func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { + // We only care about the new one, old one gets flagged as an error in unmarshal. + _, newService, err := unmarshalServices(ctx, old, new, "ValidateService") + if err != nil { + return err + } - return validateSpec(newService) + return validateSpec(newService) + } } func validateSpec(s *v1alpha1.Service) error { @@ -72,7 +75,9 @@ func validateSpec(s *v1alpha1.Service) error { return validateConfigurationSpec(&runLatest.Configuration) } -func unmarshalServices(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Service, *v1alpha1.Service, error) { +func unmarshalServices( + ctx context.Context, old GenericCRD, new GenericCRD, fnName string) (*v1alpha1.Service, *v1alpha1.Service, error) { + logger := logging.FromContext(ctx) var oldService *v1alpha1.Service if old != nil { var ok bool @@ -81,42 +86,46 @@ func unmarshalServices(old GenericCRD, new GenericCRD, fnName string) (*v1alpha1 return nil, nil, errInvalidServiceInput } } - glog.Infof("%s: OLD Service is\n%+v", fnName, oldService) + logger.Infof("%s: OLD Service is\n%+v", fnName, oldService) newService, ok := new.(*v1alpha1.Service) if !ok { return nil, nil, errInvalidServiceInput } - glog.Infof("%s: NEW Service is\n%+v", fnName, newService) + logger.Infof("%s: NEW Service is\n%+v", fnName, newService) return oldService, newService, nil } +// SetServiceDefaults set defaults on an services. // Service does not have any defaults, per-se, but because it holds a Configuration, // we need to set the Configuration's defaults. SetServiceDefaults dispatches to // SetConfigurationSpecDefaults to accomplish this. -func SetServiceDefaults(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { - _, service, err := unmarshalServices(nil, crd, "SetServiceDefaults") - if err != nil { - return err - } +func SetServiceDefaults(ctx context.Context) ResourceDefaulter { + return func(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error { + logger := logging.FromContext(ctx) + _, service, err := unmarshalServices(ctx, nil, crd, "SetServiceDefaults") + if err != nil { + return err + } - var ( - configSpec v1alpha1.ConfigurationSpec - patchBase string - ) - - if service.Spec.RunLatest != nil { - configSpec = service.Spec.RunLatest.Configuration - patchBase = "/spec/runLatest/configuration" - } else if service.Spec.Pinned != nil { - configSpec = service.Spec.Pinned.Configuration - patchBase = "/spec/pinned/configuration" - } else { - // We could error here, but validateSpec should catch this. - glog.Info("could not find config in SetServiceDefaults") - return nil - } + var ( + configSpec v1alpha1.ConfigurationSpec + patchBase string + ) + + if service.Spec.RunLatest != nil { + configSpec = service.Spec.RunLatest.Configuration + patchBase = "/spec/runLatest/configuration" + } else if service.Spec.Pinned != nil { + configSpec = service.Spec.Pinned.Configuration + patchBase = "/spec/pinned/configuration" + } else { + // We could error here, but validateSpec should catch this. + logger.Info("could not find config in SetServiceDefaults") + return nil + } - return SetConfigurationSpecDefaults(patches, patchBase, configSpec) + return setConfigurationSpecDefaults(patches, patchBase, configSpec) + } } diff --git a/pkg/webhook/service_test.go b/pkg/webhook/service_test.go index aebb5d3bcbae..3c2a8999f9fa 100644 --- a/pkg/webhook/service_test.go +++ b/pkg/webhook/service_test.go @@ -26,7 +26,7 @@ func TestEmptySpec(t *testing.T) { s := v1alpha1.Service{ Spec: v1alpha1.ServiceSpec{}, } - err := ValidateService(nil, &s, &s) + err := ValidateService(testCtx)(nil, &s, &s) if err == nil { t.Errorf("Expected failure, but succeeded with: %+v", s) } @@ -43,7 +43,7 @@ func TestRunLatest(t *testing.T) { }, }, } - if err := ValidateService(nil, &s, &s); err != nil { + if err := ValidateService(testCtx)(nil, &s, &s); err != nil { t.Errorf("Expected success, but failed with: %s", err) } } @@ -54,7 +54,7 @@ func TestRunLatestWithMissingConfiguration(t *testing.T) { RunLatest: &v1alpha1.RunLatestType{}, }, } - err := ValidateService(nil, &s, &s) + err := ValidateService(testCtx)(nil, &s, &s) if err == nil { t.Errorf("Expected failure, but succeeded with: %+v", s) } @@ -74,7 +74,7 @@ func TestPinned(t *testing.T) { }, } - if err := ValidateService(nil, &s, &s); err != nil { + if err := ValidateService(testCtx)(nil, &s, &s); err != nil { t.Errorf("Expected success, but failed with: %s", err) } } @@ -87,7 +87,7 @@ func TestPinnedFailsWithNoRevisionName(t *testing.T) { }, }, } - err := ValidateService(nil, &s, &s) + err := ValidateService(testCtx)(nil, &s, &s) if err == nil { t.Errorf("Expected failure, but succeeded with: %+v", s) } @@ -104,7 +104,7 @@ func TestPinnedFailsWithNoConfiguration(t *testing.T) { }, }, } - err := ValidateService(nil, &s, &s) + err := ValidateService(testCtx)(nil, &s, &s) if err == nil { t.Errorf("Expected failure, but succeeded with: %+v", s) } @@ -126,7 +126,7 @@ func TestPinnedSetsDefaults(t *testing.T) { s.Spec.Pinned.Configuration.RevisionTemplate.Spec.ConcurrencyModel = "" var patches []jsonpatch.JsonPatchOperation - if err := SetServiceDefaults(&patches, &s); err != nil { + if err := SetServiceDefaults(testCtx)(&patches, &s); err != nil { t.Errorf("Expected success, but failed with: %s", err) } @@ -156,7 +156,7 @@ func TestLatestSetsDefaults(t *testing.T) { s.Spec.RunLatest.Configuration.RevisionTemplate.Spec.ConcurrencyModel = "" var patches []jsonpatch.JsonPatchOperation - if err := SetServiceDefaults(&patches, &s); err != nil { + if err := SetServiceDefaults(testCtx)(&patches, &s); err != nil { t.Errorf("Expected success, but failed with: %s", err) } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index bf396c29f591..08596fa956e2 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -17,6 +17,7 @@ package webhook import ( "bytes" + "context" "crypto/tls" "crypto/x509" "encoding/json" @@ -27,10 +28,14 @@ import ( "strings" "time" + "github.com/elafros/elafros/pkg/logging/logkey" + + "go.uber.org/zap" + "github.com/elafros/elafros/pkg/apis/ela" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" + "github.com/elafros/elafros/pkg/logging" - "github.com/golang/glog" "github.com/mattbaird/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" @@ -109,6 +114,7 @@ type AdmissionController struct { client kubernetes.Interface options ControllerOptions handlers map[string]GenericCRDHandler + logger *zap.SugaredLogger } // GenericCRD is the interface definition that allows us to perform the generic @@ -161,14 +167,16 @@ func makeTLSConfig(serverCert, serverKey, caCert []byte) (*tls.Config, error) { }, nil } -func getOrGenerateKeyCertsFromSecret(client kubernetes.Interface, name, namespace string) (serverKey, serverCert, caCert []byte, err error) { +func getOrGenerateKeyCertsFromSecret(ctx context.Context, client kubernetes.Interface, name, + namespace string) (serverKey, serverCert, caCert []byte, err error) { + logger := logging.FromContext(ctx) secret, err := client.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) if err != nil { if !apierrors.IsNotFound(err) { return nil, nil, nil, err } - glog.Infof("Did not find existing secret, creating one") - newSecret, err := generateSecret(name, namespace) + logger.Info("Did not find existing secret, creating one") + newSecret, err := generateSecret(ctx, name, namespace) if err != nil { return nil, nil, nil, err } @@ -197,41 +205,43 @@ func getOrGenerateKeyCertsFromSecret(client kubernetes.Interface, name, namespac } // NewAdmissionController creates a new instance of the admission webhook controller. -func NewAdmissionController(client kubernetes.Interface, options ControllerOptions) (*AdmissionController, error) { +func NewAdmissionController(client kubernetes.Interface, options ControllerOptions, logger *zap.SugaredLogger) (*AdmissionController, error) { + ctx := logging.WithLogger(context.TODO(), logger) return &AdmissionController{ client: client, options: options, handlers: map[string]GenericCRDHandler{ "Revision": GenericCRDHandler{ Factory: &v1alpha1.Revision{}, - Defaulter: SetRevisionDefaults, - Validator: ValidateRevision, + Defaulter: SetRevisionDefaults(ctx), + Validator: ValidateRevision(ctx), }, "Configuration": GenericCRDHandler{ Factory: &v1alpha1.Configuration{}, - Defaulter: SetConfigurationDefaults, - Validator: ValidateConfiguration, + Defaulter: SetConfigurationDefaults(ctx), + Validator: ValidateConfiguration(ctx), }, "Route": GenericCRDHandler{ Factory: &v1alpha1.Route{}, - Validator: ValidateRoute, + Validator: ValidateRoute(ctx), }, "Service": GenericCRDHandler{ Factory: &v1alpha1.Service{}, - Defaulter: SetServiceDefaults, - Validator: ValidateService, + Defaulter: SetServiceDefaults(ctx), + Validator: ValidateService(ctx), }, }, + logger: logger, }, nil } -func configureCerts(client kubernetes.Interface, options *ControllerOptions) (*tls.Config, []byte, error) { +func configureCerts(ctx context.Context, client kubernetes.Interface, options *ControllerOptions) (*tls.Config, []byte, error) { apiServerCACert, err := getAPIServerExtensionCACert(client) if err != nil { return nil, nil, err } serverKey, serverCert, caCert, err := getOrGenerateKeyCertsFromSecret( - client, options.SecretName, options.ServiceNamespace) + ctx, client, options.SecretName, options.ServiceNamespace) if err != nil { return nil, nil, err } @@ -244,9 +254,11 @@ func configureCerts(client kubernetes.Interface, options *ControllerOptions) (*t // Run implements the admission controller run loop. func (ac *AdmissionController) Run(stop <-chan struct{}) error { - tlsConfig, caCert, err := configureCerts(ac.client, &ac.options) + logger := ac.logger + ctx := logging.WithLogger(context.TODO(), logger) + tlsConfig, caCert, err := configureCerts(ctx, ac.client, &ac.options) if err != nil { - glog.Infof("Could not configure admission webhook certs: %v", err) + logger.Error("Could not configure admission webhook certs", zap.Error(err)) return err } @@ -256,31 +268,31 @@ func (ac *AdmissionController) Run(stop <-chan struct{}) error { TLSConfig: tlsConfig, } - glog.Info("Found certificates for webhook...") + logger.Info("Found certificates for webhook...") if ac.options.RegistrationDelay != 0 { - glog.Infof("Delaying admission webhook registration for %v", ac.options.RegistrationDelay) + logger.Infof("Delaying admission webhook registration for %v", ac.options.RegistrationDelay) } select { case <-time.After(ac.options.RegistrationDelay): cl := ac.client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() - if err := ac.register(cl, caCert); err != nil { - glog.Infof("Failed to register webhook: %v", err) + if err := ac.register(ctx, cl, caCert); err != nil { + logger.Error("Failed to register webhook", zap.Error(err)) return err } defer func() { - if err := ac.unregister(cl); err != nil { - glog.Infof("Failed to unregister webhook: %v", err) + if err := ac.unregister(ctx, cl); err != nil { + logger.Error("Failed to unregister webhook", zap.Error(err)) } }() - glog.Info("Successfully registered webhook") + logger.Info("Successfully registered webhook") case <-stop: return nil } go func() { if err := server.ListenAndServeTLS("", ""); err != nil { - glog.Infof("ListenAndServeTLS for admission webhook returned error: %v", err) + logger.Error("ListenAndServeTLS for admission webhook returned error", zap.Error(err)) } }() <-stop @@ -289,15 +301,19 @@ func (ac *AdmissionController) Run(stop <-chan struct{}) error { } // Unregister unregisters the external admission webhook -func (ac *AdmissionController) unregister(client clientadmissionregistrationv1beta1.MutatingWebhookConfigurationInterface) error { - glog.Info("Exiting..") +func (ac *AdmissionController) unregister( + ctx context.Context, client clientadmissionregistrationv1beta1.MutatingWebhookConfigurationInterface) error { + logger := logging.FromContext(ctx) + logger.Info("Exiting..") return nil } // Register registers the external admission webhook for pilot // configuration types. -func (ac *AdmissionController) register(client clientadmissionregistrationv1beta1.MutatingWebhookConfigurationInterface, caCert []byte) error { // nolint: lll +func (ac *AdmissionController) register( + ctx context.Context, client clientadmissionregistrationv1beta1.MutatingWebhookConfigurationInterface, caCert []byte) error { // nolint: lll + logger := logging.FromContext(ctx) resources := []string{"configurations", "routes", "revisions", "services"} webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ @@ -343,23 +359,23 @@ func (ac *AdmissionController) register(client clientadmissionregistrationv1beta if !apierrors.IsAlreadyExists(err) { return fmt.Errorf("Failed to create a webhook: %s", err) } - glog.Infof("Webhook already exists") + logger.Info("Webhook already exists") configuredWebhook, err := client.Get(ac.options.WebhookName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("Error retrieving webhook: %s", err) } if !reflect.DeepEqual(configuredWebhook.Webhooks, webhook.Webhooks) { - glog.Infof("Updating webhook") + logger.Info("Updating webhook") // Set the ResourceVersion as required by update. webhook.ObjectMeta.ResourceVersion = configuredWebhook.ObjectMeta.ResourceVersion if _, err := client.Update(webhook); err != nil { return fmt.Errorf("Failed to update webhook: %s", err) } } else { - glog.Infof("Webhook is already valid") + logger.Info("Webhook is already valid") } } else { - glog.Infof("Created a webhook") + logger.Info("Created a webhook") } return nil } @@ -367,7 +383,8 @@ func (ac *AdmissionController) register(client clientadmissionregistrationv1beta // ServeHTTP implements the external admission webhook for mutating // ela resources. func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request) { - glog.Infof("Webhook ServeHTTP request=%#v", r) + logger := ac.logger + logger.Infof("Webhook ServeHTTP request=%#v", r) // verify the content type is accurate contentType := r.Header.Get("Content-Type") @@ -383,14 +400,22 @@ func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request) return } - reviewResponse := ac.admit(review.Request) + logger = logger.With( + zap.String(logkey.Kind, fmt.Sprint(review.Request.Kind)), + zap.String(logkey.Namespace, review.Request.Namespace), + zap.String(logkey.Name, review.Request.Name), + zap.String(logkey.Operation, fmt.Sprint(review.Request.Operation)), + zap.String(logkey.Resource, fmt.Sprint(review.Request.Resource)), + zap.String(logkey.SubResource, fmt.Sprint(review.Request.SubResource)), + zap.String(logkey.UserInfo, fmt.Sprint(review.Request.UserInfo))) + reviewResponse := ac.admit(logging.WithLogger(r.Context(), logger), review.Request) var response admissionv1beta1.AdmissionReview if reviewResponse != nil { response.Response = reviewResponse response.Response.UID = review.Request.UID } - glog.Infof("AdmissionReview for %s: %v/%v response=%v", + logger.Infof("AdmissionReview for %s: %v/%v response=%v", review.Request.Kind, review.Request.Namespace, review.Request.Name, reviewResponse) if err := json.NewEncoder(w).Encode(response); err != nil { @@ -407,19 +432,20 @@ func makeErrorStatus(reason string, args ...interface{}) *admissionv1beta1.Admis } } -func (ac *AdmissionController) admit(request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { +func (ac *AdmissionController) admit(ctx context.Context, request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { + logger := logging.FromContext(ctx) switch request.Operation { case admissionv1beta1.Create, admissionv1beta1.Update: default: - glog.Infof("Unhandled webhook operation, letting it through %v", request.Operation) + logger.Infof("Unhandled webhook operation, letting it through %v", request.Operation) return &admissionv1beta1.AdmissionResponse{Allowed: true} } - patchBytes, err := ac.mutate(request.Kind.Kind, request.OldObject.Raw, request.Object.Raw) + patchBytes, err := ac.mutate(ctx, request.Kind.Kind, request.OldObject.Raw, request.Object.Raw) if err != nil { return makeErrorStatus("mutation failed: %v", err) } - glog.Infof("Kind: %q PatchBytes: %v", request.Kind, string(patchBytes)) + logger.Infof("Kind: %q PatchBytes: %v", request.Kind, string(patchBytes)) return &admissionv1beta1.AdmissionResponse{ Patch: patchBytes, @@ -431,10 +457,11 @@ func (ac *AdmissionController) admit(request *admissionv1beta1.AdmissionRequest) } } -func (ac *AdmissionController) mutate(kind string, oldBytes []byte, newBytes []byte) ([]byte, error) { +func (ac *AdmissionController) mutate(ctx context.Context, kind string, oldBytes []byte, newBytes []byte) ([]byte, error) { + logger := logging.FromContext(ctx) handler, ok := ac.handlers[kind] if !ok { - glog.Warningf("Unhandled kind %q", kind) + logger.Errorf("Unhandled kind %q", kind) return nil, fmt.Errorf("unhandled kind: %q", kind) } @@ -465,15 +492,15 @@ func (ac *AdmissionController) mutate(kind string, oldBytes []byte, newBytes []b var patches []jsonpatch.JsonPatchOperation - err := updateGeneration(&patches, oldObj, newObj) + err := updateGeneration(ctx, &patches, oldObj, newObj) if err != nil { - glog.Warningf("Failed to update generation : %s", err) + logger.Error("Failed to update generation", zap.Error(err)) return nil, fmt.Errorf("Failed to update generation: %s", err) } if defaulter := handler.Defaulter; defaulter != nil { if err := defaulter(&patches, newObj); err != nil { - glog.Warningf("Failed the resource specific defaulter: %s", err) + logger.Error("Failed the resource specific defaulter", zap.Error(err)) // Return the error message as-is to give the defaulter callback // discretion over (our portion of) the message that the user sees. return nil, err @@ -481,14 +508,14 @@ func (ac *AdmissionController) mutate(kind string, oldBytes []byte, newBytes []b } if err := handler.Validator(&patches, oldObj, newObj); err != nil { - glog.Warningf("Failed the resource specific validation: %s", err) + logger.Error("Failed the resource specific validation", zap.Error(err)) // Return the error message as-is to give the validation callback // discretion over (our portion of) the message that the user sees. return nil, err } if err := validateMetadata(newObj); err != nil { - glog.Warningf("Failed to validate : %s", err) + logger.Error("Failed to validate", zap.Error(err)) return nil, fmt.Errorf("Failed to validate: %s", err) } return json.Marshal(patches) @@ -515,15 +542,16 @@ func validateMetadata(new GenericCRD) error { // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) // So, we add Generation here. Once that gets fixed, remove this and use // ObjectMeta.Generation instead. -func updateGeneration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { +func updateGeneration(ctx context.Context, patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { + logger := logging.FromContext(ctx) var oldGeneration int64 if old == nil { - glog.Infof("Old is nil") + logger.Info("Old is nil") } else { oldGeneration = old.GetGeneration() } if oldGeneration == 0 { - glog.Infof("Creating an object, setting generation to 1") + logger.Info("Creating an object, setting generation to 1") *patches = append(*patches, jsonpatch.JsonPatchOperation{ Operation: "add", Path: "/spec/generation", @@ -534,11 +562,11 @@ func updateGeneration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, n oldSpecJSON, err := old.GetSpecJSON() if err != nil { - glog.Warningf("Failed to get Spec JSON for old: %s", err) + logger.Error("Failed to get Spec JSON for old", zap.Error(err)) } newSpecJSON, err := new.GetSpecJSON() if err != nil { - glog.Warningf("Failed to get Spec JSON for new: %s", err) + logger.Error("Failed to get Spec JSON for new", zap.Error(err)) } specPatches, err := jsonpatch.CreatePatch(oldSpecJSON, newSpecJSON) @@ -549,10 +577,10 @@ func updateGeneration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, n if len(specPatches) > 0 { specPatchesJSON, err := json.Marshal(specPatches) if err != nil { - glog.Infof("Failed to marshal spec patches: %s", err) + logger.Error("Failed to marshal spec patches", zap.Error(err)) return err } - glog.Infof("Specs differ:\n%+v\n", string(specPatchesJSON)) + logger.Infof("Specs differ:\n%+v\n", string(specPatchesJSON)) operation := "replace" if newGeneration := new.GetGeneration(); newGeneration == 0 { @@ -570,12 +598,12 @@ func updateGeneration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, n }) return nil } - glog.Infof("No changes in the spec, not bumping generation...") + logger.Info("No changes in the spec, not bumping generation") return nil } -func generateSecret(name, namespace string) (*corev1.Secret, error) { - serverKey, serverCert, caCert, err := CreateCerts() +func generateSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) { + serverKey, serverCert, caCert, err := CreateCerts(ctx) if err != nil { return nil, err } diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index 3654433d2694..5637b1cd9636 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -16,13 +16,17 @@ limitations under the License. package webhook import ( + "context" "encoding/json" "fmt" "reflect" "strings" "testing" + "go.uber.org/zap" + "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" + "github.com/elafros/elafros/pkg/logging" "github.com/mattbaird/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" @@ -54,6 +58,11 @@ const ( testServiceName = "test-service-name" ) +var ( + testLogger = zap.NewNop().Sugar() + testCtx = logging.WithLogger(context.TODO(), testLogger) +) + func newRunningTestAdmissionController(t *testing.T, options ControllerOptions) ( kubeClient *fakekubeclientset.Clientset, ac *AdmissionController, @@ -61,7 +70,7 @@ func newRunningTestAdmissionController(t *testing.T, options ControllerOptions) // Create fake clients kubeClient = fakekubeclientset.NewSimpleClientset() - ac, err := NewAdmissionController(kubeClient, options) + ac, err := NewAdmissionController(kubeClient, options, testLogger) if err != nil { t.Fatalf("Failed to create new admission controller: %s", err) } @@ -81,7 +90,7 @@ func newNonRunningTestAdmissionController(t *testing.T, options ControllerOption // Create fake clients kubeClient = fakekubeclientset.NewSimpleClientset() - ac, err := NewAdmissionController(kubeClient, options) + ac, err := NewAdmissionController(kubeClient, options, testLogger) if err != nil { t.Fatalf("Failed to create new admission controller: %s", err) } @@ -95,7 +104,7 @@ func TestDeleteAllowed(t *testing.T) { Operation: admissionv1beta1.Delete, } - resp := ac.admit(&req) + resp := ac.admit(testCtx, &req) if !resp.Allowed { t.Fatalf("unexpected denial of delete") } @@ -108,7 +117,7 @@ func TestConnectAllowed(t *testing.T) { Operation: admissionv1beta1.Connect, } - resp := ac.admit(&req) + resp := ac.admit(testCtx, &req) if !resp.Allowed { t.Fatalf("unexpected denial of connect") } @@ -122,7 +131,7 @@ func TestUnknownKindFails(t *testing.T) { Kind: metav1.GroupVersionKind{Kind: "Garbage"}, } - expectFailsWith(t, ac.admit(&req), "unhandled kind") + expectFailsWith(t, ac.admit(testCtx, &req), "unhandled kind") } func TestInvalidNewConfigurationNameFails(t *testing.T) { @@ -138,7 +147,7 @@ func TestInvalidNewConfigurationNameFails(t *testing.T) { t.Fatalf("Failed to marshal configuration: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") invalidName = strings.Repeat("a", 64) config = createConfiguration(0, invalidName) @@ -147,12 +156,12 @@ func TestInvalidNewConfigurationNameFails(t *testing.T) { t.Fatalf("Failed to marshal configuration: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") } func TestValidNewConfigurationObject(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(createValidCreateConfiguration()) + resp := ac.admit(testCtx, createValidCreateConfiguration()) expectAllowed(t, resp) p := incrementGenerationPatch(0) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) @@ -162,7 +171,7 @@ func TestValidConfigurationNoChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) old := createConfiguration(testGeneration, testConfigurationName) new := createConfiguration(testGeneration, testConfigurationName) - resp := ac.admit(createUpdateConfiguration(&old, &new)) + resp := ac.admit(testCtx, createUpdateConfiguration(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } @@ -177,7 +186,7 @@ func TestValidConfigurationEnvChanges(t *testing.T) { Value: "different", }, } - resp := ac.admit(createUpdateConfiguration(&old, &new)) + resp := ac.admit(testCtx, createUpdateConfiguration(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ jsonpatch.JsonPatchOperation{ @@ -201,7 +210,7 @@ func TestInvalidNewRouteNameFails(t *testing.T) { t.Fatalf("Failed to marshal route: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") invalidName = strings.Repeat("a", 64) config = createRoute(0, invalidName) @@ -210,12 +219,12 @@ func TestInvalidNewRouteNameFails(t *testing.T) { t.Fatalf("Failed to marshal route: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") } func TestValidNewRouteObject(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(createValidCreateRoute()) + resp := ac.admit(testCtx, createValidCreateRoute()) expectAllowed(t, resp) p := incrementGenerationPatch(0) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) @@ -225,7 +234,7 @@ func TestValidRouteNoChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) old := createRoute(1, testRouteName) new := createRoute(1, testRouteName) - resp := ac.admit(createUpdateRoute(&old, &new)) + resp := ac.admit(testCtx, createUpdateRoute(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } @@ -238,7 +247,7 @@ func TestInvalidOldRoute(t *testing.T) { t.Errorf("Marshal(%v) = %v", new, err) } oldBytes := []byte(`{"bad": "field"}`) - resp := ac.admit(createUpdateRouteRaw(oldBytes, newBytes)) + resp := ac.admit(testCtx, createUpdateRouteRaw(oldBytes, newBytes)) expectFailsWith(t, resp, `unknown field "bad"`) } @@ -250,7 +259,7 @@ func TestInvalidNewRoute(t *testing.T) { t.Errorf("Marshal(%v) = %v", old, err) } newBytes := []byte(`{"sepc": {}}`) - resp := ac.admit(createUpdateRouteRaw(oldBytes, newBytes)) + resp := ac.admit(testCtx, createUpdateRouteRaw(oldBytes, newBytes)) expectFailsWith(t, resp, `unknown field "sepc"`) } @@ -264,7 +273,7 @@ func TestValidRouteChanges(t *testing.T) { Percent: 100, }, } - resp := ac.admit(createUpdateRoute(&old, &new)) + resp := ac.admit(testCtx, createUpdateRoute(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ jsonpatch.JsonPatchOperation{ @@ -288,7 +297,7 @@ func TestValidNewRevisionObject(t *testing.T) { t.Fatalf("Failed to marshal revision: %s", err) } req.Object.Raw = marshaled - resp := ac.admit(req) + resp := ac.admit(testCtx, req) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ jsonpatch.JsonPatchOperation{ @@ -326,7 +335,7 @@ func TestValidRevisionUpdates(t *testing.T) { t.Fatalf("Failed to marshal revision: %s", err) } req.Object.Raw = marshaled - resp := ac.admit(req) + resp := ac.admit(testCtx, req) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ jsonpatch.JsonPatchOperation{ @@ -360,7 +369,7 @@ func TestInvalidRevisionUpdate(t *testing.T) { } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Revision spec should not change") + expectFailsWith(t, ac.admit(testCtx, req), "Revision spec should not change") } func TestInvalidNewRevisionNameFails(t *testing.T) { @@ -377,7 +386,7 @@ func TestInvalidNewRevisionNameFails(t *testing.T) { t.Fatalf("Failed to marshal revision: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") invalidName = strings.Repeat("a", 64) revision = createRevision(invalidName) @@ -387,12 +396,12 @@ func TestInvalidNewRevisionNameFails(t *testing.T) { t.Fatalf("Failed to marshal revision: %s", err) } req.Object.Raw = marshaled - expectFailsWith(t, ac.admit(req), "Invalid resource name") + expectFailsWith(t, ac.admit(testCtx, req), "Invalid resource name") } func TestValidNewServicePinned(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(createValidCreateServicePinned()) + resp := ac.admit(testCtx, createValidCreateServicePinned()) expectAllowed(t, resp) p := incrementGenerationPatch(0) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) @@ -400,7 +409,7 @@ func TestValidNewServicePinned(t *testing.T) { func TestValidNewServiceRunLatest(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(createValidCreateServiceRunLatest()) + resp := ac.admit(testCtx, createValidCreateServiceRunLatest()) expectAllowed(t, resp) p := incrementGenerationPatch(0) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) @@ -410,14 +419,14 @@ func TestInvalidNewServiceNoSpecs(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) svc := createServicePinned(0, testServiceName) svc.Spec.Pinned = nil - expectFailsWith(t, ac.admit(createCreateService(svc)), "exactly one of runLatest or pinned") + expectFailsWith(t, ac.admit(testCtx, createCreateService(svc)), "exactly one of runLatest or pinned") } func TestInvalidNewServiceNoRevisionNameInPinned(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) svc := createServicePinned(0, testServiceName) svc.Spec.Pinned.RevisionName = "" - expectFailsWith(t, ac.admit(createCreateService(svc)), "spec.pinned.revisionName") + expectFailsWith(t, ac.admit(testCtx, createCreateService(svc)), "spec.pinned.revisionName") } func TestValidServiceEnvChanges(t *testing.T) { @@ -430,7 +439,7 @@ func TestValidServiceEnvChanges(t *testing.T) { Value: "different", }, } - resp := ac.admit(createUpdateService(&old, &new)) + resp := ac.admit(testCtx, createUpdateService(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ jsonpatch.JsonPatchOperation{ @@ -444,7 +453,7 @@ func TestValidServiceEnvChanges(t *testing.T) { func TestValidWebhook(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) createDeployment(ac) - ac.register(ac.client.Admissionregistration().MutatingWebhookConfigurations(), []byte{}) + ac.register(testCtx, ac.client.Admissionregistration().MutatingWebhookConfigurations(), []byte{}) _, err := ac.client.Admissionregistration().MutatingWebhookConfigurations().Get(ac.options.WebhookName, metav1.GetOptions{}) if err != nil { t.Fatalf("Failed to create webhook: %s", err) @@ -468,7 +477,7 @@ func TestUpdatingWebhook(t *testing.T) { createDeployment(ac) createWebhook(ac, webhook) - ac.register(ac.client.Admissionregistration().MutatingWebhookConfigurations(), []byte{}) + ac.register(testCtx, ac.client.Admissionregistration().MutatingWebhookConfigurations(), []byte{}) currentWebhook, _ := ac.client.Admissionregistration().MutatingWebhookConfigurations().Get(ac.options.WebhookName, metav1.GetOptions{}) if reflect.DeepEqual(currentWebhook.Webhooks, webhook.Webhooks) { t.Fatalf("Expected webhook to be updated")