Browse Source

cmd/gitops-pusher: re-use existing types from acl package

This changes the ACLTestError type to reuse the existing/identical
types from the ACL implementation, to avoid issues in the future if
the two types fall out of sync.

Updates #8645

Signed-off-by: Jenny Zhang <[email protected]>
Jenny Zhang 2 years ago
parent
commit
9ab70212f4
2 changed files with 78 additions and 14 deletions
  1. 23 14
      cmd/gitops-pusher/gitops-pusher.go
  2. 55 0
      cmd/gitops-pusher/gitops-pusher_test.go

+ 23 - 14
cmd/gitops-pusher/gitops-pusher.go

@@ -23,6 +23,7 @@ import (
 	"github.com/peterbourgon/ff/v3/ffcli"
 	"github.com/tailscale/hujson"
 	"golang.org/x/oauth2/clientcredentials"
+	"tailscale.com/client/tailscale"
 	"tailscale.com/util/httpm"
 )
 
@@ -270,7 +271,7 @@ func applyNewACL(ctx context.Context, client *http.Client, tailnet, apiKey, poli
 	got := resp.StatusCode
 	want := http.StatusOK
 	if got != want {
-		var ate ACLTestError
+		var ate ACLGitopsTestError
 		err := json.NewDecoder(resp.Body).Decode(&ate)
 		if err != nil {
 			return err
@@ -306,7 +307,7 @@ func testNewACLs(ctx context.Context, client *http.Client, tailnet, apiKey, poli
 	}
 	defer resp.Body.Close()
 
-	var ate ACLTestError
+	var ate ACLGitopsTestError
 	err = json.NewDecoder(resp.Body).Decode(&ate)
 	if err != nil {
 		return err
@@ -327,12 +328,12 @@ func testNewACLs(ctx context.Context, client *http.Client, tailnet, apiKey, poli
 
 var lineColMessageSplit = regexp.MustCompile(`line ([0-9]+), column ([0-9]+): (.*)$`)
 
-type ACLTestError struct {
-	Message string               `json:"message"`
-	Data    []ACLTestErrorDetail `json:"data"`
+// ACLGitopsTestError is redefined here so we can add a custom .Error() response
+type ACLGitopsTestError struct {
+	tailscale.ACLTestError
 }
 
-func (ate ACLTestError) Error() string {
+func (ate ACLGitopsTestError) Error() string {
 	var sb strings.Builder
 
 	if *githubSyntax && lineColMessageSplit.MatchString(ate.Message) {
@@ -349,20 +350,28 @@ func (ate ACLTestError) Error() string {
 	fmt.Fprintln(&sb)
 
 	for _, data := range ate.Data {
-		fmt.Fprintf(&sb, "For user %s:\n", data.User)
-		for _, err := range data.Errors {
-			fmt.Fprintf(&sb, "- %s\n", err)
+		if data.User != "" {
+			fmt.Fprintf(&sb, "For user %s:\n", data.User)
+		}
+
+		if len(data.Errors) > 0 {
+			fmt.Fprint(&sb, "Errors found:\n")
+			for _, err := range data.Errors {
+				fmt.Fprintf(&sb, "- %s\n", err)
+			}
+		}
+
+		if len(data.Warnings) > 0 {
+			fmt.Fprint(&sb, "Warnings found:\n")
+			for _, err := range data.Warnings {
+				fmt.Fprintf(&sb, "- %s\n", err)
+			}
 		}
 	}
 
 	return sb.String()
 }
 
-type ACLTestErrorDetail struct {
-	User   string   `json:"user"`
-	Errors []string `json:"errors"`
-}
-
 func getACLETag(ctx context.Context, client *http.Client, tailnet, apiKey string) (string, error) {
 	req, err := http.NewRequestWithContext(ctx, httpm.GET, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl", *apiServer, tailnet), nil)
 	if err != nil {

+ 55 - 0
cmd/gitops-pusher/gitops-pusher_test.go

@@ -0,0 +1,55 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+package main
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"tailscale.com/client/tailscale"
+)
+
+func TestEmbeddedTypeUnmarshal(t *testing.T) {
+	var gitopsErr ACLGitopsTestError
+	gitopsErr.Message = "gitops response error"
+	gitopsErr.Data = []tailscale.ACLTestFailureSummary{
+		{
+			User:   "GitopsError",
+			Errors: []string{"this was initially created as a gitops error"},
+		},
+	}
+
+	var aclTestErr tailscale.ACLTestError
+	aclTestErr.Message = "native ACL response error"
+	aclTestErr.Data = []tailscale.ACLTestFailureSummary{
+		{
+			User:   "ACLError",
+			Errors: []string{"this was initially created as an ACL error"},
+		},
+	}
+
+	t.Run("unmarshal gitops type from acl type", func(t *testing.T) {
+		b, _ := json.Marshal(aclTestErr)
+		var e ACLGitopsTestError
+		err := json.Unmarshal(b, &e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !strings.Contains(e.Error(), "For user ACLError") { // the gitops error prints out the user, the acl error doesn't
+			t.Fatalf("user heading for 'ACLError' not found in gitops error: %v", e.Error())
+		}
+	})
+	t.Run("unmarshal acl type from gitops type", func(t *testing.T) {
+		b, _ := json.Marshal(gitopsErr)
+		var e tailscale.ACLTestError
+		err := json.Unmarshal(b, &e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		expectedErr := `Status: 0, Message: "gitops response error", Data: [{User:GitopsError Errors:[this was initially created as a gitops error] Warnings:[]}]`
+		if e.Error() != expectedErr {
+			t.Fatalf("got %v\n, expected %v", e.Error(), expectedErr)
+		}
+	})
+}