Skip to content

Commit

Permalink
PR: documentation and organization
Browse files Browse the repository at this point in the history
Moved scrubbing under `internal/log`.
Added proper godocs to exported functions

Signed-off-by: Hamza El-Saawy <[email protected]>
  • Loading branch information
helsaawy committed Mar 8, 2022
1 parent 1574549 commit 7eaa713
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 55 deletions.
2 changes: 1 addition & 1 deletion cmd/containerd-shim-runhcs-v1/options/runhcs.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/containerd-shim-runhcs-v1/options/runhcs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ message Options {
// UTC.
bool no_inherit_host_timezone = 19;

// scrub_logs enables removing environment variables and other protentially sensitive information form logs
// scrub_logs enables removing environment variables and other protentially sensitive information from logs
bool scrub_logs = 20;
}

Expand Down
13 changes: 7 additions & 6 deletions cmd/containerd-shim-runhcs-v1/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ import (
"unsafe"

"github.com/Microsoft/go-winio"
runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/internal/extendedtask"
"github.com/Microsoft/hcsshim/internal/scrub"
"github.com/Microsoft/hcsshim/internal/shimdiag"
"github.com/Microsoft/hcsshim/pkg/octtrpc"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/runtime/v2/task"
"github.com/containerd/ttrpc"
Expand All @@ -27,6 +22,12 @@ import (
"github.com/sirupsen/logrus"
"github.com/urfave/cli"
"golang.org/x/sys/windows"

runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/internal/extendedtask"
hcslog "github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/shimdiag"
"github.com/Microsoft/hcsshim/pkg/octtrpc"
)

var svc *service
Expand Down Expand Up @@ -161,7 +162,7 @@ var serveCommand = cli.Command{

// enable scrubbing
if shimOpts.ScrubLogs {
scrub.SetScrubbing(true)
hcslog.SetScrubbing(true)
}

// Force the cli.ErrWriter to be os.Stdout for this. We use stderr for
Expand Down
7 changes: 4 additions & 3 deletions internal/gcs/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import (
"syscall"
"time"

"github.com/Microsoft/hcsshim/internal/scrub"
"github.com/sirupsen/logrus"
"golang.org/x/sys/windows"

"github.com/Microsoft/hcsshim/internal/log"
)

const (
Expand Down Expand Up @@ -400,9 +401,9 @@ func (brdg *bridge) writeMessage(buf *bytes.Buffer, enc *json.Encoder, typ msgTy
switch typ {
// container environment vars are in rpCreate for linux; rpcExecuteProcess for windows
case msgType(rpcCreate) | msgTypeRequest:
b, err = scrub.BridgeCreate(b)
b, err = log.ScrubBridgeCreate(b)
case msgType(rpcExecuteProcess) | msgTypeRequest:
b, err = scrub.BridgeExecProcess(b)
b, err = log.ScrubBridgeExecProcess(b)
}
if err != nil {
brdg.log.WithError(err).Warning("could not scrub bridge payload")
Expand Down
32 changes: 16 additions & 16 deletions internal/scrub/scrub.go → internal/log/scrub.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// This package scrubs objects of customer information to pass to logging
package scrub
package log

import (
"bytes"
Expand All @@ -10,7 +9,7 @@ import (
hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
)

// scrubbing utilities to remove user information from arbitrary objects
// This package scrubs objects of potentially sensitive information to pass to logging

type genMap = map[string]interface{}
type scrubberFunc func(genMap) error
Expand All @@ -35,18 +34,18 @@ func SetScrubbing(enable bool) {
atomic.StoreInt32(&_scrub, v)
}

// ScrubbingEnabled checks if scrubbing is enabled
func ScrubbingEnabled() bool {
// IsScrubbingEnabled checks if scrubbing is enabled
func IsScrubbingEnabled() bool {
v := atomic.LoadInt32(&_scrub)
return v != 0
}

// for HCS Create Process requests with config parameters of
// type internal/hcs/schema2.ProcessParameters (aka hcsshema.ProcessParameters)
func ProcessParameters(s string) (string, error) {
// ScrubProcessParameters scrubs HCS Create Process requests with config parameters of
// type internal/hcs/schema2.ScrubProcessParameters (aka hcsshema.ScrubProcessParameters)
func ScrubProcessParameters(s string) (string, error) {
// todo: deal with v1 ProcessConfig
b := []byte(s)
if !ScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
return s, nil
}

Expand All @@ -63,9 +62,9 @@ func ProcessParameters(s string) (string, error) {
return buf.String(), nil
}

// for requests sent over the bridge of type
// internal/gcs/protocol.containerCreate wrapping a internal/hcsoci.linuxHostedSystem
func BridgeCreate(b []byte) ([]byte, error) {
// ScrubBridgeCreate scrubs requests sent over the bridge of type
// internal/gcs/protocol.containerCreate wrapping an internal/hcsoci.linuxHostedSystem
func ScrubBridgeCreate(b []byte) ([]byte, error) {
return scrubBytes(b, scrubLinuxHostedSystem)
}

Expand All @@ -86,8 +85,9 @@ func scrubLinuxHostedSystem(m genMap) error {
return ErrUnknownType
}

// for requests sent over the bridge of type internal/gcs/protocol.containerExecuteProcess
func BridgeExecProcess(b []byte) ([]byte, error) {
// ScrubBridgeExecProcess scrubs requests sent over the bridge of type
// internal/gcs/protocol.containerExecuteProcess
func ScrubBridgeExecProcess(b []byte) ([]byte, error) {
return scrubBytes(b, scrubExecuteProcess)
}

Expand All @@ -103,7 +103,7 @@ func scrubExecuteProcess(m genMap) error {
return ErrUnknownType
}

s, err := ProcessParameters(s)
s, err := ScrubProcessParameters(s)
if err != nil {
return err
}
Expand All @@ -116,7 +116,7 @@ func scrubExecuteProcess(m genMap) error {
}

func scrubBytes(b []byte, scrub scrubberFunc) ([]byte, error) {
if !ScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
if !IsScrubbingEnabled() || !hasKeywords(b) || !json.Valid(b) {
return b, nil
}

Expand Down
6 changes: 3 additions & 3 deletions internal/vmcompute/vmcompute.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import (
"syscall"
"time"

"go.opencensus.io/trace"

"github.com/Microsoft/hcsshim/internal/interop"
"github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/logfields"
"github.com/Microsoft/hcsshim/internal/oc"
"github.com/Microsoft/hcsshim/internal/scrub"
"github.com/Microsoft/hcsshim/internal/timeout"
"go.opencensus.io/trace"
)

//go:generate go run ../../mksyscall_windows.go -output zsyscall_windows.go vmcompute.go
Expand Down Expand Up @@ -392,7 +392,7 @@ func HcsCreateProcess(ctx gcontext.Context, computeSystem HcsSystem, processPara
}()
if span.IsRecordingEvents() {
// wont handle v1 process parameters
if s, err := scrub.ProcessParameters(processParameters); err == nil {
if s, err := log.ScrubProcessParameters(processParameters); err == nil {
span.AddAttributes(trace.StringAttribute("processParameters", s))
}
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion test/vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ github.com/Microsoft/hcsshim/internal/resources
github.com/Microsoft/hcsshim/internal/runhcs
github.com/Microsoft/hcsshim/internal/safefile
github.com/Microsoft/hcsshim/internal/schemaversion
github.com/Microsoft/hcsshim/internal/scrub
github.com/Microsoft/hcsshim/internal/shimdiag
github.com/Microsoft/hcsshim/internal/timeout
github.com/Microsoft/hcsshim/internal/uvm
Expand Down

0 comments on commit 7eaa713

Please sign in to comment.