From 25e19f4a2cf72c2a7f4cde4880f69780d0e1e404 Mon Sep 17 00:00:00 2001 From: Linus Arver Date: Fri, 6 Mar 2020 18:44:59 -0800 Subject: [PATCH 1/3] ValidateThinManifestDirectoryStructure: do not pollute STDOUT This affects the flag -manifest-based-snapshot-of flag. --- lib/dockerregistry/inventory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dockerregistry/inventory.go b/lib/dockerregistry/inventory.go index fe35cd90..21cba0df 100644 --- a/lib/dockerregistry/inventory.go +++ b/lib/dockerregistry/inventory.go @@ -302,7 +302,7 @@ func ValidateThinManifestDirectoryStructure( return err } - fmt.Printf("*looking at %q\n", dir) + klog.Infof("*looking at %q", dir) for _, file := range files { p, err := os.Stat(filepath.Join(manifestDir, file.Name())) if err != nil { From fce1c7142e8216125c5190e238cd19d8c0ed8aad Mon Sep 17 00:00:00 2001 From: Linus Arver Date: Fri, 6 Mar 2020 18:55:20 -0800 Subject: [PATCH 2/3] audit.go: move types to separate file No functional change. --- lib/audit/BUILD.bazel | 5 +++- lib/audit/auditor.go | 31 ----------------------- lib/audit/types.go | 57 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 lib/audit/types.go diff --git a/lib/audit/BUILD.bazel b/lib/audit/BUILD.bazel index 0af3eee4..1dd6a4b4 100644 --- a/lib/audit/BUILD.bazel +++ b/lib/audit/BUILD.bazel @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["auditor.go"], + srcs = [ + "auditor.go", + "types.go", + ], importpath = "sigs.k8s.io/k8s-container-image-promoter/lib/audit", visibility = ["//visibility:public"], deps = [ diff --git a/lib/audit/auditor.go b/lib/audit/auditor.go index a6bd71cb..d5d450aa 100644 --- a/lib/audit/auditor.go +++ b/lib/audit/auditor.go @@ -36,37 +36,6 @@ import ( reg "sigs.k8s.io/k8s-container-image-promoter/lib/dockerregistry" ) -// ServerContext holds all of the initialization data for the server to start -// up. -type ServerContext struct { - ID string - RepoURL *url.URL - RepoBranch string - ThinManifestDirPath string - ErrorReportingClient *errorreporting.Client - LogClient *logging.Client -} - -// PubSubMessageInner is the inner struct that holds the actual Pub/Sub -// information. -type PubSubMessageInner struct { - Data []byte `json:"data,omitempty"` - ID string `json:"id"` -} - -// PubSubMessage is the payload of a Pub/Sub event. -type PubSubMessage struct { - Message PubSubMessageInner `json:"message"` - Subscription string `json:"subscription"` -} - -const ( - cloneDepth = 1 - // LogName is the auditing log name to use. This is the name that comes up - // for "gcloud logging logs list". - LogName = "cip-audit-log" -) - func initServerContext( gcpProjectID, repoURLStr, branch, path, uuid string, ) (*ServerContext, error) { diff --git a/lib/audit/types.go b/lib/audit/types.go new file mode 100644 index 00000000..da15597c --- /dev/null +++ b/lib/audit/types.go @@ -0,0 +1,57 @@ +/* +Copyright 2020 The Kubernetes 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 + + 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. +*/ + +package audit + +import ( + "net/url" + + "cloud.google.com/go/errorreporting" + "cloud.google.com/go/logging" +) + +// ServerContext holds all of the initialization data for the server to start +// up. +type ServerContext struct { + ID string + RepoURL *url.URL + RepoBranch string + ThinManifestDirPath string + // TODO: Change ErrorReportingClient and LogClient into interfaces. Then use + // dependency injection to make unit tests for the Audit() function. + ErrorReportingClient *errorreporting.Client + LogClient *logging.Client +} + +// PubSubMessageInner is the inner struct that holds the actual Pub/Sub +// information. +type PubSubMessageInner struct { + Data []byte `json:"data,omitempty"` + ID string `json:"id"` +} + +// PubSubMessage is the payload of a Pub/Sub event. +type PubSubMessage struct { + Message PubSubMessageInner `json:"message"` + Subscription string `json:"subscription"` +} + +const ( + cloneDepth = 1 + // LogName is the auditing log name to use. This is the name that comes up + // for "gcloud logging logs list". + LogName = "cip-audit-log" +) From 0e4c23fdd8d0762b7a1a60acef9d7db774dea382 Mon Sep 17 00:00:00 2001 From: Linus Arver Date: Mon, 9 Mar 2020 14:39:41 -0700 Subject: [PATCH 3/3] ServerContext: use a LoggingFacility wrapper for logs Instead of hardcoding the logging client tied to GCP resources, use a "LoggingFacility" wrapper so that we can in the future use a fake logging client for unit tests. --- lib/audit/BUILD.bazel | 2 +- lib/audit/auditor.go | 31 +++++----------------- lib/audit/types.go | 14 +++++----- lib/logclient/BUILD.bazel | 16 ++++++++++++ lib/logclient/fake.go | 40 +++++++++++++++++++++++++++++ lib/logclient/gcp.go | 54 +++++++++++++++++++++++++++++++++++++++ lib/logclient/types.go | 50 ++++++++++++++++++++++++++++++++++++ 7 files changed, 174 insertions(+), 33 deletions(-) create mode 100644 lib/logclient/BUILD.bazel create mode 100644 lib/logclient/fake.go create mode 100644 lib/logclient/gcp.go create mode 100644 lib/logclient/types.go diff --git a/lib/audit/BUILD.bazel b/lib/audit/BUILD.bazel index 1dd6a4b4..f8976ebe 100644 --- a/lib/audit/BUILD.bazel +++ b/lib/audit/BUILD.bazel @@ -10,8 +10,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//lib/dockerregistry:go_default_library", + "//lib/logclient:go_default_library", "@com_google_cloud_go//errorreporting:go_default_library", - "@com_google_cloud_go_logging//:go_default_library", "@in_gopkg_src_d_go_git_v4//:go_default_library", "@in_gopkg_src_d_go_git_v4//plumbing:go_default_library", "@io_k8s_klog//:go_default_library", diff --git a/lib/audit/auditor.go b/lib/audit/auditor.go index d5d450aa..a1fccc52 100644 --- a/lib/audit/auditor.go +++ b/lib/audit/auditor.go @@ -29,11 +29,11 @@ import ( "strings" "cloud.google.com/go/errorreporting" - "cloud.google.com/go/logging" git "gopkg.in/src-d/go-git.v4" "gopkg.in/src-d/go-git.v4/plumbing" "k8s.io/klog" reg "sigs.k8s.io/k8s-container-image-promoter/lib/dockerregistry" + "sigs.k8s.io/k8s-container-image-promoter/lib/logclient" ) func initServerContext( @@ -46,7 +46,7 @@ func initServerContext( } erc := initErrorReportingClient(gcpProjectID) - logClient := initLogClient(gcpProjectID) + loggingFacility := logclient.NewGcpLoggingFacility(gcpProjectID, LogName) serverContext := ServerContext{ ID: uuid, @@ -54,29 +54,12 @@ func initServerContext( RepoBranch: branch, ThinManifestDirPath: path, ErrorReportingClient: erc, - LogClient: logClient, + LoggingFacility: loggingFacility, } return &serverContext, nil } -// initLogClient creates a logging client that performs better logging than the -// default behavior on GCP Stackdriver. For instance, logs sent with this client -// are not split up over newlines, and also the severity levels are actually -// understood by Stackdriver. -func initLogClient(projectID string) *logging.Client { - - ctx := context.Background() - - // Creates a client. - client, err := logging.NewClient(ctx, projectID) - if err != nil { - klog.Fatalf("Failed to create client: %v", err) - } - - return client -} - func initErrorReportingClient(projectID string) *errorreporting.Client { ctx := context.Background() @@ -106,7 +89,7 @@ func Auditor(gcpProjectID, repoURL, branch, path, uuid string) { klog.Infoln(serverContext) // nolint[errcheck] - defer serverContext.LogClient.Close() + defer serverContext.LoggingFacility.Close() // nolint[errcheck] defer serverContext.ErrorReportingClient.Close() @@ -246,9 +229,9 @@ func ParsePubSubMessage(r *http.Request) (*reg.GCRPubSubPayload, error) { // other. // nolint[funlen] func (s *ServerContext) Audit(w http.ResponseWriter, r *http.Request) { - logInfo := s.LogClient.Logger(LogName).StandardLogger(logging.Info) - logError := s.LogClient.Logger(LogName).StandardLogger(logging.Error) - logAlert := s.LogClient.Logger(LogName).StandardLogger(logging.Alert) + logInfo := s.LoggingFacility.LogInfo + logError := s.LoggingFacility.LogError + logAlert := s.LoggingFacility.LogAlert defer func() { if msg := recover(); msg != nil { diff --git a/lib/audit/types.go b/lib/audit/types.go index da15597c..d1903872 100644 --- a/lib/audit/types.go +++ b/lib/audit/types.go @@ -20,20 +20,18 @@ import ( "net/url" "cloud.google.com/go/errorreporting" - "cloud.google.com/go/logging" + "sigs.k8s.io/k8s-container-image-promoter/lib/logclient" ) // ServerContext holds all of the initialization data for the server to start // up. type ServerContext struct { - ID string - RepoURL *url.URL - RepoBranch string - ThinManifestDirPath string - // TODO: Change ErrorReportingClient and LogClient into interfaces. Then use - // dependency injection to make unit tests for the Audit() function. + ID string + RepoURL *url.URL + RepoBranch string + ThinManifestDirPath string ErrorReportingClient *errorreporting.Client - LogClient *logging.Client + LoggingFacility *logclient.LoggingFacility } // PubSubMessageInner is the inner struct that holds the actual Pub/Sub diff --git a/lib/logclient/BUILD.bazel b/lib/logclient/BUILD.bazel new file mode 100644 index 00000000..0adf152d --- /dev/null +++ b/lib/logclient/BUILD.bazel @@ -0,0 +1,16 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = [ + "fake.go", + "gcp.go", + "types.go", + ], + importpath = "sigs.k8s.io/k8s-container-image-promoter/lib/logclient", + visibility = ["//visibility:public"], + deps = [ + "@com_google_cloud_go_logging//:go_default_library", + "@io_k8s_klog//:go_default_library", + ], +) diff --git a/lib/logclient/fake.go b/lib/logclient/fake.go new file mode 100644 index 00000000..680c453f --- /dev/null +++ b/lib/logclient/fake.go @@ -0,0 +1,40 @@ +/* +Copyright 2020 The Kubernetes 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 + + 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. +*/ + +package logclient + +import ( + "log" + "os" +) + +// FakeLogClient is a fake log client. Its sole purpose is to implement a NOP +// "Close()" method, for tests. +type FakeLogClient struct{} + +// Close is a NOP (there is nothing to close). +func (fakeLogClient *FakeLogClient) Close() error { return nil } + +// NewFakeLoggingFacility returns a new LoggingFacility, but whose resources are +// all local (stdout), with a FakeLogClient. +func NewFakeLoggingFacility() *LoggingFacility { + + logInfo := log.New(os.Stdout, "FAKE-INFO", log.LstdFlags) + logError := log.New(os.Stdout, "FAKE-ERROR", log.LstdFlags) + logAlert := log.New(os.Stdout, "FAKE-ALERT", log.LstdFlags) + + return New(logInfo, logError, logAlert, &FakeLogClient{}) +} diff --git a/lib/logclient/gcp.go b/lib/logclient/gcp.go new file mode 100644 index 00000000..8dad3490 --- /dev/null +++ b/lib/logclient/gcp.go @@ -0,0 +1,54 @@ +/* +Copyright 2020 The Kubernetes 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 + + 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. +*/ + +package logclient + +import ( + "context" + + "cloud.google.com/go/logging" + "k8s.io/klog" +) + +// NewGcpLoggingFacility returns a new LoggingFacility that logs to GCP +// resources. As such, it requires the GCP projectID as well as the logName to +// log to. +func NewGcpLoggingFacility(projectID, logName string) *LoggingFacility { + gcpLogClient := initGcpLogClient(projectID) + + logInfo := gcpLogClient.Logger(logName).StandardLogger(logging.Info) + logError := gcpLogClient.Logger(logName).StandardLogger(logging.Error) + logAlert := gcpLogClient.Logger(logName).StandardLogger(logging.Alert) + + return New(logInfo, logError, logAlert, gcpLogClient) +} + +// initGcpLogClient creates a logging client that performs better logging than +// the default behavior on GCP Stackdriver. For instance, logs sent with this +// client are not split up over newlines, and also the severity levels are +// actually understood by Stackdriver. +func initGcpLogClient(projectID string) *logging.Client { + + ctx := context.Background() + + // Creates a client. + client, err := logging.NewClient(ctx, projectID) + if err != nil { + klog.Fatalf("Failed to create client: %v", err) + } + + return client +} diff --git a/lib/logclient/types.go b/lib/logclient/types.go new file mode 100644 index 00000000..7a084429 --- /dev/null +++ b/lib/logclient/types.go @@ -0,0 +1,50 @@ +/* +Copyright 2020 The Kubernetes 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 + + 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. +*/ + +package logclient + +import ( + "io" + "log" +) + +// LoggingFacility bundles 3 loggers together. +type LoggingFacility struct { + LogInfo *log.Logger + LogError *log.Logger + LogAlert *log.Logger + + logClient io.Closer +} + +// New returns a new LoggingFacility, based on the given loggers. +func New( + logInfo, logError, logAlert *log.Logger, + logClient io.Closer, +) *LoggingFacility { + return &LoggingFacility{ + LogInfo: logInfo, + LogError: logError, + LogAlert: logAlert, + logClient: logClient, + } +} + +// Close implements the "Close" method for the LoggingFacility. This just calls +// Close() on the "logClient". +func (loggingFacility *LoggingFacility) Close() error { + return loggingFacility.logClient.Close() +}