Merge pull request #153 from farhan-pasha/farhan/fix-go-report-card

I am merging this. We’ll still need to run tests on the build server to verify everything is in good shape. Will do that either over the weekend or Monday…ish.
vmware-tanzu · Aug 10, 2023 · 252c408 · 252c408
2 parents acfaef6 + d990a07
commit 252c408
Show file tree

Hide file tree

Showing 3 changed files with 134 additions and 163 deletions.
diff --git a/app/safe/internal/server/route/fetch.go b/app/safe/internal/server/route/fetch.go
@@ -13,18 +13,115 @@ package route
 import (
  "encoding/json"
  "fmt"
+ "io"
+ "net/http"
+ "strings"
+ "time"
+
  "github.com/vmware-tanzu/secrets-manager/app/safe/internal/state"
  "github.com/vmware-tanzu/secrets-manager/core/audit"
+ v1 "github.com/vmware-tanzu/secrets-manager/core/entity/data/v1"
  reqres "github.com/vmware-tanzu/secrets-manager/core/entity/reqres/safe/v1"
  "github.com/vmware-tanzu/secrets-manager/core/env"
  "github.com/vmware-tanzu/secrets-manager/core/log"
  "github.com/vmware-tanzu/secrets-manager/core/validation"
- "io"
- "net/http"
- "strings"
- "time"
 )
 
+func handleBadSvidResponse(cid string, w http.ResponseWriter, svid string, j audit.JournalEntry) {
+ j.Event = audit.EventBadSvid
+ audit.Log(j)
+
+ log.DebugLn(&cid, "Fetch: bad svid", svid)
+
+ w.WriteHeader(http.StatusBadRequest)
+ _, err := io.WriteString(w, "")
+ if err != nil {
+ log.InfoLn(&cid, "Fetch: Problem sending response", err.Error())
+ }
+}
+
+func handleBadPeerSvidResponse(cid string, w http.ResponseWriter, svid string, j audit.JournalEntry) {
+ j.Event = audit.EventBadPeerSvid
+ audit.Log(j)
+
+ w.WriteHeader(http.StatusBadRequest)
+ _, err := io.WriteString(w, "")
+ if err != nil {
+ log.InfoLn(&cid, "Fetch: Problem with svid", svid)
+ }
+}
+
+func handleNoSecretResponse(cid string, w http.ResponseWriter, j audit.JournalEntry) {
+ j.Event = audit.EventNoSecret
+ audit.Log(j)
+
+ w.WriteHeader(http.StatusNotFound)
+ _, err2 := io.WriteString(w, "")
+ if err2 != nil {
+ log.InfoLn(&cid, "Fetch: Problem sending response", err2.Error())
+ }
+}
+
+func handleSuccessResponse(cid string, w http.ResponseWriter, j audit.JournalEntry, sfr reqres.SecretFetchResponse) {
+ j.Event = audit.EventOk
+ j.Entity = sfr
+ audit.Log(j)
+
+ resp, err := json.Marshal(sfr)
+ if err != nil {
+ w.WriteHeader(http.StatusInternalServerError)
+ _, err2 := io.WriteString(w, "Problem unmarshaling response")
+ if err2 != nil {
+ log.InfoLn(&cid, "Fetch: Problem sending response", err2.Error())
+ }
+ return
+ }
+
+ log.DebugLn(&cid, "Fetch: before response")
+
+ _, err = io.WriteString(w, string(resp))
+ if err != nil {
+ log.InfoLn(&cid, "Problem sending response", err.Error())
+ }
+
+ log.DebugLn(&cid, "Fetch: after response")
+}
+
+func getWorkloadIDAndParts(svid string) (string, []string) {
+ tmp := strings.Replace(svid, env.WorkloadSvidPrefix(), "", 1)
+ parts := strings.Split(tmp, "/")
+ if len(parts) > 0 {
+ return parts[0], parts
+ }
+ return "", nil
+}
+
+func getSecretValue(cid string, secret *v1.SecretStored) string {
+ if secret.ValueTransformed != "" {
+ log.TraceLn(&cid, "Fetch: using transformed value")
+ return secret.ValueTransformed
+ }
+
+ // This part is for backwards compatibility.
+ // It probably won’t execute because `secret.ValueTransformed` will
+ // always be set.
+
+ log.TraceLn(&cid, "Fetch: using raw value")
+
+ if len(secret.Values) == 1 {
+ return secret.Values[0]
+ }
+
+ jsonData, err := json.Marshal(secret.Values)
+ if err != nil {
+ log.WarnLn(&cid, "Fetch: Problem marshaling values", err.Error())
+ } else {
+ return string(jsonData)
+ }
+
+ return ""
+}
+
 func Fetch(cid string, w http.ResponseWriter, r *http.Request, svid string) {
  if env.SafeManualKeyInput() && !state.MasterKeySet() {
  log.InfoLn(&cid, "Fetch: Master key not set")
@@ -44,17 +141,7 @@ func Fetch(cid string, w http.ResponseWriter, r *http.Request, svid string) {
 
  // Only workloads can fetch.
  if !validation.IsWorkload(svid) {
- j.Event = audit.EventBadSvid
- audit.Log(j)
-
- log.DebugLn(&cid, "Fetch: bad svid", svid)
-
- w.WriteHeader(http.StatusBadRequest)
- _, err := io.WriteString(w, "")
- if err != nil {
- log.InfoLn(&cid, "Fetch: Problem sending response", err.Error())
- }
-
+ handleBadSvidResponse(cid, w, svid, j)
  return
  }
 
@@ -69,21 +156,12 @@ func Fetch(cid string, w http.ResponseWriter, r *http.Request, svid string) {
 
  log.DebugLn(&cid, "Fetch: preparing request")
 
- tmp := strings.Replace(svid, env.WorkloadSvidPrefix(), "", 1)
- parts := strings.Split(tmp, "/")
+ workloadId, parts := getWorkloadIDAndParts(svid)
  if len(parts) == 0 {
- j.Event = audit.EventBadPeerSvid
- audit.Log(j)
-
- w.WriteHeader(http.StatusBadRequest)
- _, err := io.WriteString(w, "")
- if err != nil {
- log.InfoLn(&cid, "Fetch: Problem with svid", svid)
- }
+ handleBadPeerSvidResponse(cid, w, svid, j)
  return
  }
 
- workloadId := parts[0]
  secret, err := state.ReadSecret(cid, workloadId)
  if err != nil {
  log.WarnLn(&cid, "Fetch: Problem reading secret", err.Error())
@@ -93,41 +171,13 @@ func Fetch(cid string, w http.ResponseWriter, r *http.Request, svid string) {
 
  // If secret does not exist, send an empty response.
  if secret == nil {
- j.Event = audit.EventNoSecret
- audit.Log(j)
-
- w.WriteHeader(http.StatusNotFound)
- _, err2 := io.WriteString(w, "")
- if err2 != nil {
- log.InfoLn(&cid, "Fetch: Problem sending response", err2.Error())
- }
+ handleNoSecretResponse(cid, w, j)
  return
  }
 
  log.DebugLn(&cid, "Fetch: will send. workload id:", workloadId)
 
- value := ""
- if secret.ValueTransformed != "" {
- log.TraceLn(&cid, "Fetch: using transformed value")
- value = secret.ValueTransformed
- } else {
- // This part is for backwards compatibility.
- // It probably won’t execute because `secret.ValueTransformed` will
- // always be set.
-
- log.TraceLn(&cid, "Fetch: using raw value")
-
- if len(secret.Values) == 1 {
- value = secret.Values[0]
- } else {
- jsonData, err2 := json.Marshal(secret.Values)
- if err2 != nil {
- log.WarnLn(&cid, "Fetch: Problem marshaling values", err2.Error())
- } else {
- value = string(jsonData)
- }
- }
- }
+ value := getSecretValue(cid, secret)
 
  // RFC3339 is what Go uses internally when marshaling dates.
  // Choosing it to be consistent.
@@ -137,26 +187,6 @@ func Fetch(cid string, w http.ResponseWriter, r *http.Request, svid string) {
  Updated: fmt.Sprintf("\"%s\"", secret.Updated.Format(time.RFC3339)),
  }
 
- j.Event = audit.EventOk
- j.Entity = sfr
- audit.Log(j)
+ handleSuccessResponse(cid, w, j, sfr)
 
- resp, err := json.Marshal(sfr)
- if err != nil {
- w.WriteHeader(http.StatusInternalServerError)
- _, err2 := io.WriteString(w, "Problem unmarshaling response")
- if err2 != nil {
- log.InfoLn(&cid, "Fetch: Problem sending response", err2.Error())
- }
- return
- }
-
- log.DebugLn(&cid, "Fetch: before response")
-
- _, err = io.WriteString(w, string(resp))
- if err != nil {
- log.InfoLn(&cid, "Problem sending response", err.Error())
- }
-
- log.DebugLn(&cid, "Fetch: after response")
 }
diff --git a/app/sentinel/cmd/main.go b/app/sentinel/cmd/main.go
@@ -12,9 +12,10 @@ package main
 
 import (
  "fmt"
+ "os"
+
  "github.com/akamensky/argparse"
  "github.com/vmware-tanzu/secrets-manager/app/sentinel/internal/safe"
- "os"
 )
 
 func parseList(parser *argparse.Parser) *bool {
@@ -126,69 +127,27 @@ func printSecretNeeded() {
  fmt.Println("")
 }
 
-func doPost(workload *string, secret *string, namespace *string,
- backingStore *string, useKubernetes *bool, template *string, format *string,
- encrypt *bool, deleteSecret *bool, appendSecret *bool, inputKeys *string,
-) {
- workloadP := ""
- if workload != nil {
- workloadP = *workload
- }
-
- secretP := ""
- if secret != nil {
- secretP = *secret
- }
+func inputValidationFailure(workload *string, encrypt *bool, inputKeys *string, secret *string, deleteSecret *bool) bool {
 
- namespaceP := ""
- if namespace != nil {
- namespaceP = *namespace
- }
-
- backingStoreP := ""
- if backingStore != nil {
- backingStoreP = *backingStore
- }
-
- useK8sP := false
- if useKubernetes != nil {
- useK8sP = *useKubernetes
- }
-
- templateP := ""
- if template != nil {
- templateP = *template
- }
-
- formatP := ""
- if format != nil {
- formatP = *format
- }
-
- encryptP := false
- if encrypt != nil {
- encryptP = *encrypt
- }
-
- deleteP := false
- if deleteSecret != nil {
- deleteP = *deleteSecret
- }
-
- appendP := false
- if appendSecret != nil {
- appendP = *appendSecret
+ // You need to provide a workload name if you are not encrypting a secret,
+ // or if you are not providing input keys.
+ if *workload == "" &&
+ !*encrypt &&
+ *inputKeys == "" {
+ printWorkloadNameNeeded()
+ return true
  }
 
- inputKeysP := ""
- if inputKeys != nil {
- inputKeysP = *inputKeys
+ // You need to provide a secret value if you are not deleting a secret,
+ // or if you are not providing input keys.
+ if *secret == "" &&
+ !*deleteSecret &&
+ *inputKeys == "" {
+ printSecretNeeded()
+ return true
  }
 
- safe.Post(
- workloadP, secretP, namespaceP, backingStoreP, useK8sP,
- templateP, formatP, encryptP, deleteP, appendP, inputKeysP,
- )
+ return false
 }
 
 func main() {
@@ -213,39 +172,21 @@ func main() {
  return
  }
 
- if list != nil && *list == true {
+ if *list {
  safe.Get()
  return
  }
 
- // You need to provide a workload name if you are not encrypting a secret,
- // or if you are not providing input keys.
- if (workload == nil || *workload == "") &&
- (encrypt == nil || !*encrypt) &&
- (inputKeys == nil || *inputKeys == "") {
- printWorkloadNameNeeded()
- return
- }
-
- // You need to provide a secret value if you are not deleting a secret,
- // or if you are not providing input keys.
- if (secret == nil || *secret == "") &&
- (deleteSecret == nil || !*deleteSecret) &&
- (inputKeys == nil || *inputKeys == "") {
- printSecretNeeded()
- return
- }
-
- if namespace == nil || *namespace == "" {
+ if *namespace == "" {
  *namespace = "default"
  }
 
- if inputKeys == nil || *inputKeys == "" {
- *inputKeys = ""
+ if inputValidationFailure(workload, encrypt, inputKeys, secret, deleteSecret) {
+ return
  }
 
- doPost(workload, secret, namespace, backingStore,
- useKubernetes, template, format, encrypt, deleteSecret, appendSecret,
- inputKeys,
+ safe.Post(
+ *workload, *secret, *namespace, *backingStore, *useKubernetes,
+ *template, *format, *encrypt, *deleteSecret, *appendSecret, *inputKeys,
  )
 }
diff --git a/docs/_pages/0050-quickstart.md b/docs/_pages/0050-quickstart.md
@@ -166,7 +166,7 @@ example-68997489c6-8j8kj 1/1 Running 0 1m51s
 Let’s check the logs of our example workload:
 
 ```bash
-kubectl get logs example-68997489c6-8j8kj
+kubectl logs example-68997489c6-8j8kj
 ```
 
 The output will be something similar to this:
@@ -362,7 +362,7 @@ Since we’ve registered a secret, let’s see if our example workload can fetch
 the secret now and display it in its logs.
 
 ```bash
-kubectl get logs example-68997489c6-8j8kj
+kubectl logs example-68997489c6-8j8kj
 ```
 
 And the output would be something like this: