From 7f9a2419002006bf82219b1d24a03abad0909917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Engin=20A=C3=A7=C4=B1kg=C3=B6z?= Date: Wed, 28 Aug 2024 01:10:12 +0300 Subject: [PATCH] Refactor: fix potential panic & refactor some code (#1123) * Fix potential panic Signed-off-by: Engin Acikgoz * Add test Signed-off-by: Engin Acikgoz * Simplify error generators Signed-off-by: Engin Acikgoz * No need to nil check, length check is enough Signed-off-by: Engin Acikgoz * Remove unreachable code Signed-off-by: Engin Acikgoz * Revert "Remove unreachable code" This reverts commit d0b1eafcab52a0cf40cb878fb73680bf69639b9e. Signed-off-by: Engin Acikgoz --- app/sentinel/internal/oidc/engine/adapter.go | 2 +- app/sentinel/internal/oidc/safe/get.go | 2 +- app/sentinel/internal/oidc/safe/impl.go | 2 +- app/sentinel/internal/oidc/safe/post.go | 2 +- ci/test/assert/assert.go | 4 +-- core/crypto/encrypt.go | 7 +----- core/entity/v1/data/convert.go | 2 +- core/entity/v1/data/convert_test.go | 26 ++++++++++++++++++++ 8 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 core/entity/v1/data/convert_test.go diff --git a/app/sentinel/internal/oidc/engine/adapter.go b/app/sentinel/internal/oidc/engine/adapter.go index 98a0b3bb..a2f8ca8c 100644 --- a/app/sentinel/internal/oidc/engine/adapter.go +++ b/app/sentinel/internal/oidc/engine/adapter.go @@ -102,7 +102,7 @@ func HandleCommandSecrets( return } - if req.Namespaces == nil || len(req.Namespaces) == 0 { + if len(req.Namespaces) == 0 { req.Namespaces = []string{"default"} } diff --git a/app/sentinel/internal/oidc/safe/get.go b/app/sentinel/internal/oidc/safe/get.go index 672038d6..4b1d6700 100644 --- a/app/sentinel/internal/oidc/safe/get.go +++ b/app/sentinel/internal/oidc/safe/get.go @@ -72,7 +72,7 @@ func Get( } }(source) if !proceed { - return "", fmt.Errorf("could not proceed") + return "", errors.New("could not proceed") } authorizer := tlsconfig.AdaptMatcher(func(id spiffeid.ID) error { diff --git a/app/sentinel/internal/oidc/safe/impl.go b/app/sentinel/internal/oidc/safe/impl.go index a5c99171..8da50541 100644 --- a/app/sentinel/internal/oidc/safe/impl.go +++ b/app/sentinel/internal/oidc/safe/impl.go @@ -87,7 +87,7 @@ func newSecretUpsertRequest( func respond(cid *string, r *http.Response) (string, error) { if r == nil { - return "", fmt.Errorf("post: Response is null") + return "", errors.New("post: Response is null") } defer func(b io.ReadCloser) { diff --git a/app/sentinel/internal/oidc/safe/post.go b/app/sentinel/internal/oidc/safe/post.go index 6a8b9427..f291db74 100644 --- a/app/sentinel/internal/oidc/safe/post.go +++ b/app/sentinel/internal/oidc/safe/post.go @@ -98,7 +98,7 @@ func Post( log.Println(cid, "Post: I cannot execute command because I cannot talk to SPIRE.") return "", - fmt.Errorf( + errors.New( "post: I cannot execute command because I cannot talk to SPIRE") } diff --git a/ci/test/assert/assert.go b/ci/test/assert/assert.go index 476d9b99..0a68684f 100644 --- a/ci/test/assert/assert.go +++ b/ci/test/assert/assert.go @@ -102,9 +102,7 @@ func WorkloadIsRunning() error { } if podCount != 1 { - return errors.New( - fmt.Sprintf("Expected 1 running pod for workload, found %d", podCount), - ) + return fmt.Errorf("Expected 1 running pod for workload, found %d", podCount) } return nil diff --git a/core/crypto/encrypt.go b/core/crypto/encrypt.go index a4b61220..f68d6468 100644 --- a/core/crypto/encrypt.go +++ b/core/crypto/encrypt.go @@ -97,12 +97,7 @@ func EncryptToWriterAge(out io.Writer, data string) error { defer func(w io.WriteCloser) { err := w.Close() if err != nil { - fmt.Println( - fmt.Sprintf( - "encryptToWriterAge: problem closing stream: %s", - err.Error(), - ), - ) + fmt.Printf("encryptToWriterAge: problem closing stream: %s\n", err.Error()) } }(wrappedWriter) diff --git a/core/entity/v1/data/convert.go b/core/entity/v1/data/convert.go index 6e293514..d5a79970 100644 --- a/core/entity/v1/data/convert.go +++ b/core/entity/v1/data/convert.go @@ -62,7 +62,7 @@ func convertValueToMap(values []string) map[string][]byte { // otherwise, it returns a map with a single entry, "VALUE", containing the // original 'value' as []byte. func convertValueNoTemplate(values []string) map[string][]byte { - var data map[string][]byte + var data = make(map[string][]byte) var jsonData map[string]string val := "" diff --git a/core/entity/v1/data/convert_test.go b/core/entity/v1/data/convert_test.go new file mode 100644 index 00000000..80d1db25 --- /dev/null +++ b/core/entity/v1/data/convert_test.go @@ -0,0 +1,26 @@ +package data + +import ( + "reflect" + "testing" +) + +func TestConvertValueNoTemplate(t *testing.T) { + t.Run("Success", func(t *testing.T) { + values := []string{"{\"key\":\"value\"}"} + expected := map[string][]byte{"key": []byte("value")} + actual := convertValueNoTemplate(values) + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected %v but got %v", expected, actual) + } + }) + + t.Run("Failure", func(t *testing.T) { + values := []string{"key:value"} + expected := map[string][]byte{"VALUE": []byte("key:value")} + actual := convertValueNoTemplate(values) + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected %v but got %v", expected, actual) + } + }) +}