Skip to content

Commit

Permalink
Address gosec findings
Browse files Browse the repository at this point in the history
  • Loading branch information
SaschaSchwarze0 committed Oct 4, 2023
1 parent 73505c4 commit 3b9fbe2
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 30 deletions.
5 changes: 0 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,3 @@ linters:
- misspell
disable:
- errcheck

linters-settings:
gosec:
excludes:
- G101 # Potential hardcoded credentials
9 changes: 6 additions & 3 deletions cmd/shp/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ var hiddenLogFlags = []string{
}

func main() {
initGoFlags()
if err := initGoFlags(); err != nil {
fmt.Fprintf(os.Stderr, "ERROR: %v\n", err)
os.Exit(1)
}
initPFlags()

streams := genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}
Expand All @@ -48,7 +51,7 @@ func main() {

// initGoFlags initializes the flag sets for klog.
// Any flags for "-h" or "--help" are ignored because pflag will show the usage later with all subcommands.
func initGoFlags() {
func initGoFlags() error {
flagset := goflag.NewFlagSet(ApplicationName, goflag.ContinueOnError)
goflag.CommandLine = flagset
klog.InitFlags(flagset)
Expand All @@ -59,7 +62,7 @@ func initGoFlags() {
args = append(args, arg)
}
}
flagset.Parse(args)
return flagset.Parse(args)
}

// initPFlags initializes the pflags used by Cobra subcommands.
Expand Down
4 changes: 1 addition & 3 deletions pkg/shp/cmd/build/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,5 @@ func (c *ListCommand) Run(params *params.Params, io *genericclioptions.IOStreams
fmt.Fprintf(writer, columnTemplate, b.Name, b.Spec.Output.Image, message)
}

writer.Flush()

return nil
return writer.Flush()
}
4 changes: 2 additions & 2 deletions pkg/shp/cmd/build/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestStartBuildRunFollowLog(t *testing.T) {
},
}

for _, test := range tests {
for i, test := range tests {
name := "testpod"
containerName := "container"
pod := &corev1.Pod{
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestStartBuildRunFollowLog(t *testing.T) {
cmd.Cmd().ExecuteC()
pm := genericclioptions.NewConfigFlags(true)
if len(test.to) > 0 {
pm.Timeout = &test.to
pm.Timeout = &tests[i].to
}
failureDuration := 1 * time.Millisecond
param := params.NewParamsForTest(kclientset, shpclientset, pm, metav1.NamespaceDefault, &failureDuration, &failureDuration)
Expand Down
4 changes: 1 addition & 3 deletions pkg/shp/cmd/buildrun/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,5 @@ func (c *ListCommand) Run(params *params.Params, _ *genericclioptions.IOStreams)
fmt.Fprintf(writer, columnTemplate, name, status, age)
}

writer.Flush()

return nil
return writer.Flush()
}
4 changes: 2 additions & 2 deletions pkg/shp/cmd/buildrun/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestStreamBuildRunFollowLogs(t *testing.T) {
},
}

for _, test := range tests {
for i, test := range tests {
name := "testpod"
containerName := "container"
pod := &corev1.Pod{
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestStreamBuildRunFollowLogs(t *testing.T) {
cmd.Cmd().ExecuteC()
pm := genericclioptions.NewConfigFlags(true)
if len(test.to) > 0 {
pm.Timeout = &test.to
pm.Timeout = &tests[i].to
}
param := params.NewParamsForTest(kclientset, shpclientset, pm, metav1.NamespaceDefault, nil, nil)

Expand Down
2 changes: 1 addition & 1 deletion pkg/shp/cmd/follower/follow.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (f *Follower) OnNoPodEventsYet(podList *corev1.PodList) {
f.Log(fmt.Sprintf("BuildRun %q log following has not observed any pod events yet.\n", f.buildRun.Name))
if podList != nil && len(podList.Items) > 0 {
f.Log(fmt.Sprintf("BuildRun %q's Pod completed before the log following's watch was established.\n", f.buildRun.Name))
f.OnEvent(&podList.Items[0])
f.OnEvent(&podList.Items[0]) // #nosec G104 there is nothing we must handle here, the error is logged in the function already
return
}
brClient := f.buildClientset.ShipwrightV1alpha1().BuildRuns(f.buildRun.Namespace)
Expand Down
4 changes: 2 additions & 2 deletions pkg/shp/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const (
// SourceContextDirFlag command-line flag.
SourceContextDirFlag = "source-context-dir"
// SourceCredentialsSecretFlag command-line flag.
SourceCredentialsSecretFlag = "source-credentials-secret"
SourceCredentialsSecretFlag = "source-credentials-secret" // #nosec G101
// SourceBundleImageFlag command-line flag
SourceBundleImageFlag = "source-bundle-image"
// SourceBundlePruneFlag command-line flag
Expand All @@ -45,7 +45,7 @@ const (
// OutputInsecure command-line flag.
OutputInsecureFlag = "output-insecure"
// OutputCredentialsSecretFlag command-line flag.
OutputCredentialsSecretFlag = "output-credentials-secret"
OutputCredentialsSecretFlag = "output-credentials-secret" // #nosec G101
// ServiceAccountNameFlag command-line flag.
ServiceAccountNameFlag = "sa-name"
// ServiceAccountGenerateFlag command-line flag.
Expand Down
3 changes: 1 addition & 2 deletions pkg/shp/params/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ func (p *Params) ShipwrightClientSet() (buildclientset.Interface, error) {
return p.buildClientset, nil
}

// Namespace returns kubernetes namespace with alle the overrides
// Namespace returns kubernetes namespace with all the overrides
// from command line and kubernetes config
func (p *Params) Namespace() string {
if len(p.namespace) == 0 {
clientConfig := p.configFlags.ToRawKubeConfigLoader()
clientConfig.ClientConfig()
p.namespace, _, _ = clientConfig.Namespace()

}
Expand Down
8 changes: 5 additions & 3 deletions pkg/shp/streamer/tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,17 @@ func (t *Tar) skipPath(fpath string, stat fs.FileInfo) bool {
// Create the actual tar by inspecting all files in source path, skipping some.
func (t *Tar) Create(w io.Writer) error {
tw := tar.NewWriter(w)
filepath.Walk(t.src, func(fpath string, stat fs.FileInfo, err error) error {
if err := filepath.Walk(t.src, func(fpath string, stat fs.FileInfo, err error) error {
if err != nil {
return err
}
if t.skipPath(fpath, stat) {
return nil
}
return writeFileToTar(tw, t.src, fpath, stat)
})
}); err != nil {
return err
}
return tw.Close()
}

Expand Down Expand Up @@ -99,6 +101,6 @@ func (t *Tar) tarSize() error {
return nil
})

t.Size = size+size*1/100
t.Size = size + size*1/100
return err
}
1 change: 1 addition & 0 deletions pkg/shp/streamer/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func writeFileToTar(tw *tar.Writer, src, fpath string, stat fs.FileInfo) error {
return err
}

// #nosec G304 intentionally opening file from variable
f, err := os.Open(fpath)
if err != nil {
return err
Expand Down
10 changes: 8 additions & 2 deletions pkg/shp/tail/tail.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,17 @@ func (t *Tail) Start(ns, podName, container string) {
fmt.Fprintln(t.stderr, err)
return
}
defer stream.Close()
defer func() {
if err := stream.Close(); err != nil {
fmt.Fprintf(t.stderr, "Failed to close stream: %v", err)
}
}()

go func() {
<-t.stopCh
stream.Close()
if err := stream.Close(); err != nil {
fmt.Fprintf(t.stderr, "Failed to close stream: %v", err)
}
}()

containerName := strings.TrimPrefix(container, "step-")
Expand Down
5 changes: 4 additions & 1 deletion test/mock/fake_clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package mock
import (
"bytes"
"io"
"log"
"net/http"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -58,7 +59,9 @@ func (f *FakeClientset) roundTripperFn(req *http.Request) (*http.Response, error
// bootstrap instantiate the basic elements of the clientset.
func (f *FakeClientset) bootstrap() {
f.scheme = runtime.NewScheme()
f.scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{})
if err := f.scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{}); err != nil {
log.Fatal(err)
}
f.scheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Pod{}, &metav1.Status{})

f.codecs = serializer.NewCodecFactory(f.scheme)
Expand Down
6 changes: 5 additions & 1 deletion test/stub/client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package stub

import (
"log"

buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"

"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -11,6 +13,8 @@ import (
// NewFakeClient creates a fake client with Shipwright's Build scheme.
func NewFakeClient() dynamic.Interface {
scheme := runtime.NewScheme()
buildv1alpha1.SchemeBuilder.AddToScheme(scheme)
if err := buildv1alpha1.SchemeBuilder.AddToScheme(scheme); err != nil {
log.Fatal(err)
}
return fake.NewSimpleDynamicClient(scheme)
}

0 comments on commit 3b9fbe2

Please sign in to comment.