Browse Source

Fix testing flags showing up in release binaries (#15797)

* Move test helper to test package

* add test

* Skip tests in codecov

* Revert "Move test helper to test package"

This reverts commit 1ed6cec3fe.

* Replace testing with interface
John Howard 1 month ago
parent
commit
a918e38f97
5 changed files with 50 additions and 11 deletions
  1. 3
    4
      Makefile
  2. 1
    1
      codecov.skip
  3. 8
    3
      pilot/pkg/security/authz/policy/helper.go
  4. 1
    1
      prow/istio-unit-tests.sh
  5. 37
    2
      tests/binary/binaries_test.go

+ 3
- 4
Makefile View File

@@ -427,10 +427,9 @@ build: depend $(BUILD_BINS)
427 427
 .PHONY: build-linux
428 428
 build-linux: depend $(LINUX_BUILD_BINS)
429 429
 
430
-.PHONY: version-test
431
-# Do not run istioctl since is different (connects to kubernetes)
432
-version-test:
433
-	go test ./tests/version/... -v --base-dir ${ISTIO_OUT} --binaries="$(PILOT_BINS) mixc mixs mixgen node_agent node_agent_k8s istio_ca galley sdsclient"
430
+.PHONY: binaries-test
431
+binaries-test:
432
+	go test ./tests/binary/... -v --base-dir ${ISTIO_OUT} --binaries="$(BUILD_BINS)"
434 433
 
435 434
 # The following are convenience aliases for most of the go targets
436 435
 # The first block is for aliases that are the same as the actual binary,

+ 1
- 1
codecov.skip View File

@@ -18,7 +18,7 @@ istio.io/istio/samples
18 18
 istio.io/istio/security/tests/integration
19 19
 istio.io/istio/tests/codecov
20 20
 istio.io/istio/tests/e2e
21
-istio.io/istio/tests/version
21
+istio.io/istio/tests/binary
22 22
 istio.io/istio/tests/integration2/examples
23 23
 istio.io/istio/tests/integration2/qualification
24 24
 istio.io/istio/tests/integration_old

+ 8
- 3
pilot/pkg/security/authz/policy/helper.go View File

@@ -17,7 +17,6 @@ package policy
17 17
 import (
18 18
 	"fmt"
19 19
 	"strings"
20
-	"testing"
21 20
 
22 21
 	"github.com/davecgh/go-spew/spew"
23 22
 	envoy_rbac "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v2"
@@ -30,7 +29,13 @@ import (
30 29
 	"istio.io/istio/pkg/config"
31 30
 )
32 31
 
33
-func NewServiceMetadata(hostname string, labels map[string]string, t *testing.T) *authz_model.ServiceMetadata {
32
+// We cannot import `testing` here, as it will bring extra test flags into the binary. Instead, just include the interface here
33
+type mockTest interface {
34
+	Fatalf(format string, args ...interface{})
35
+	Helper()
36
+}
37
+
38
+func NewServiceMetadata(hostname string, labels map[string]string, t mockTest) *authz_model.ServiceMetadata {
34 39
 	t.Helper()
35 40
 	splits := strings.Split(hostname, ".")
36 41
 	if len(splits) < 2 {
@@ -58,7 +63,7 @@ func NewServiceMetadata(hostname string, labels map[string]string, t *testing.T)
58 63
 	return serviceMetadata
59 64
 }
60 65
 
61
-func NewAuthzPolicies(policies []*model.Config, t *testing.T) *model.AuthorizationPolicies {
66
+func NewAuthzPolicies(policies []*model.Config, t mockTest) *model.AuthorizationPolicies {
62 67
 	t.Helper()
63 68
 
64 69
 	hasClusterRbacConfig := false

+ 1
- 1
prow/istio-unit-tests.sh View File

@@ -34,4 +34,4 @@ cd "${ROOT}"
34 34
 # Integration/e2e tests in the other scripts are run against GKE or real clusters.
35 35
 export JUNIT_UNIT_TEST_XML="${ARTIFACTS}/junit_unit-tests.xml"
36 36
 export T="-v"
37
-make build localTestEnv test version-test
37
+make build localTestEnv test binaries-test

tests/version/version_test.go → tests/binary/binaries_test.go View File

@@ -12,7 +12,7 @@
12 12
 // See the License for the specific language governing permissions and
13 13
 // limitations under the License.
14 14
 
15
-package version
15
+package binary
16 16
 
17 17
 import (
18 18
 	"encoding/json"
@@ -45,7 +45,12 @@ func TestVersion(t *testing.T) {
45 45
 	for _, b := range binariesToTest {
46 46
 		cmd := path.Join(*releasedir, b)
47 47
 		t.Run(b, func(t *testing.T) {
48
-			out, err := exec.Command(cmd, "version", "-ojson").Output()
48
+			args := []string{"version", "-ojson"}
49
+			if b == "istioctl" {
50
+				args = append(args, "--remote=false")
51
+			}
52
+
53
+			out, err := exec.Command(cmd, args...).Output()
49 54
 			if err != nil {
50 55
 				t.Fatalf("Version failed with error: %v. Output: %v", err, string(out))
51 56
 			}
@@ -69,6 +74,36 @@ func TestVersion(t *testing.T) {
69 74
 	}
70 75
 }
71 76
 
77
+var (
78
+	// If this flag is present, it means "testing" was imported by code that is built by the binary
79
+	blacklistedFlags = []string{
80
+		"--test.memprofilerate",
81
+	}
82
+)
83
+
84
+// Test that flags do not get polluted with unexpected flags
85
+func TestFlags(t *testing.T) {
86
+	binariesToTest := strings.Split(*binaries, " ")
87
+	if len(binariesToTest) == 0 {
88
+		t.Fatal("No binaries to test. Pass the --binaries flag.")
89
+	}
90
+	for _, b := range binariesToTest {
91
+		cmd := path.Join(*releasedir, b)
92
+		t.Run(b, func(t *testing.T) {
93
+			out, err := exec.Command(cmd, "--help").Output()
94
+			if err != nil {
95
+				t.Fatalf("Version failed with error: %v. Output: %v", err, string(out))
96
+			}
97
+
98
+			for _, blacklist := range blacklistedFlags {
99
+				if strings.Contains(string(out), blacklist) {
100
+					t.Fatalf("binary contains unexpected flags: %v", string(out))
101
+				}
102
+			}
103
+		})
104
+	}
105
+}
106
+
72 107
 // TODO we may be able to do more validation of fields here, but because it changes based on the environment this is not easy
73 108
 // For now ensuring the fields even get populated is most important.
74 109
 func validateField(t *testing.T, field, s string) {

Loading…
Cancel
Save