Omit gateway tags if empty (#4127)

* Omit gateway tags if empty

* Fix existing omit tags case in Fill

* Remove some gatewayTags invalid values in tests

* Remove second omitEmpty case for gateway tags

* Remove allocation of Tags, not required

* Disable duplicate test for gatewayTags disabled

* Verify gatewayTags array count with schema

* Implement furkans suggestions as per PR #4129

* Add test case for Fill/ExtractTo behaviour

* Update API schema (go generate)

* Add test to validate GatewayTags in Tyk extension

* Return enabled:false in oas document, don't omit

* Adjust gatewayTags tests to account for ShouldOmit

* Remove test, we test for gwtags fill in apidef

* Remove duplicate tests for gatewaytags

* Remove duplicate tests for gatewaytags

* Remove gatewayTags.Tags minItems=1 from validation, pass along as-is, fix tests

(cherry picked from commit f86f019)

apidef/oas/oas_test.go

@@ -13,39 +13,40 @@ import (
 func TestOAS(t *testing.T) {
 	t.Parallel()
 
-	var nilOASPaths OAS
-
-	var emptyOASPaths OAS
-	xTykAPIGateway := &XTykAPIGateway{}
-	Fill(t, &xTykAPIGateway.Server.GatewayTags, 0)
-
-	emptyOASPaths.SetTykExtension(xTykAPIGateway)
-	emptyOASPaths.Paths = make(openapi3.Paths)
-
-	nilOASPaths.SetTykExtension(xTykAPIGateway)
-
 	t.Run("empty paths", func(t *testing.T) {
 		t.Parallel()
 
+		var emptyOASPaths OAS
+		emptyOASPaths.Paths = make(openapi3.Paths)
+		emptyOASPaths.SetTykExtension(&XTykAPIGateway{})
+
 		var convertedAPI apidef.APIDefinition
 		emptyOASPaths.ExtractTo(&convertedAPI)
 
 		var resultOAS OAS
 		resultOAS.Fill(convertedAPI)
 
+		// This tests that zero-value extensions are cleared
+		emptyOASPaths.Extensions = nil
 		assert.Equal(t, emptyOASPaths, resultOAS)
 	})
 
 	t.Run("nil paths", func(t *testing.T) {
 		t.Parallel()
 
+		var nilOASPaths OAS
+		nilOASPaths.SetTykExtension(&XTykAPIGateway{})
+
 		var convertedAPI apidef.APIDefinition
 		nilOASPaths.ExtractTo(&convertedAPI)
 
 		var resultOAS OAS
 		resultOAS.Fill(convertedAPI)
 
-		assert.Equal(t, emptyOASPaths, resultOAS)
+		// No paths in base OAS produce empty paths{} when converted back
+		nilOASPaths.Paths = make(openapi3.Paths)
+		nilOASPaths.Extensions = nil
+		assert.Equal(t, nilOASPaths, resultOAS)
 	})
 
 	t.Run("auth configs", func(t *testing.T) {

apidef/oas/root_test.go

@@ -16,7 +16,6 @@ import (
 func TestXTykAPIGateway(t *testing.T) {
 	t.Run("empty", func(t *testing.T) {
 		var emptyXTykAPIGateway XTykAPIGateway
-		Fill(t, &emptyXTykAPIGateway.Server.GatewayTags, 0)
 
 		var convertedAPI apidef.APIDefinition
 		emptyXTykAPIGateway.ExtractTo(&convertedAPI)

apidef/oas/schema/x-tyk-api-gateway.json

@@ -608,6 +608,7 @@
         },
         "tags": {
           "type": "array",
+          "uniqueItems": true,
           "items": [
             {
               "type": "string"

apidef/oas/security_test.go

@@ -418,19 +418,13 @@ func TestOAS_OAuth(t *testing.T) {
 						securityName: &oauth,
 					},
 				},
-				GatewayTags: &GatewayTags{
-					Enabled: true,
-					Tags:    []string{},
-				},
 			},
 		},
 	}
 
 	var api apidef.APIDefinition
 	oas.ExtractTo(&api)
 
-	assert.False(t, api.TagsDisabled)
-
 	var convertedOAS OAS
 	convertedOAS.Components.SecuritySchemes = oas.Components.SecuritySchemes
 	convertedOAS.Fill(api)
@@ -540,9 +534,6 @@ func TestOAS_TykAuthentication_NoOASSecurity(t *testing.T) {
 				Authentication: &Authentication{
 					HMAC: &hmac,
 				},
-				GatewayTags: &GatewayTags{
-					Enabled: true,
-				},
 			},
 		},
 	}

apidef/oas/server.go

@@ -60,8 +60,10 @@ func (s *Server) ExtractTo(api *apidef.APIDefinition) {
 	}
 	if s.GatewayTags != nil {
 		s.GatewayTags.ExtractTo(api)
+	} else {
+		api.TagsDisabled = true
+		api.Tags = []string{}
 	}
-
 	if s.CustomDomain != nil {
 		s.CustomDomain.ExtractTo(api)
 	}
@@ -114,17 +116,14 @@ type GatewayTags struct {
 
 func (gt *GatewayTags) Fill(api apidef.APIDefinition) {
 	gt.Enabled = !api.TagsDisabled
-	if api.Tags == nil {
-		api.Tags = []string{}
-	}
 	gt.Tags = api.Tags
+	if gt.Tags == nil {
+		gt.Tags = []string{}
+	}
 }
 
 func (gt *GatewayTags) ExtractTo(api *apidef.APIDefinition) {
 	api.TagsDisabled = !gt.Enabled
-	if gt.Tags == nil {
-		gt.Tags = []string{}
-	}
 	api.Tags = gt.Tags
 }
 

apidef/oas/server_test.go

@@ -1,6 +1,7 @@
 package oas
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/TykTechnologies/tyk/apidef"
@@ -11,7 +12,6 @@ func TestServer(t *testing.T) {
 	t.Parallel()
 
 	var emptyServer Server
-	Fill(t, &emptyServer.GatewayTags, 0)
 
 	var convertedAPI apidef.APIDefinition
 	emptyServer.ExtractTo(&convertedAPI)
@@ -36,6 +36,80 @@ func TestListenPath(t *testing.T) {
 	assert.Equal(t, emptyListenPath, resultListenPath)
 }
 
+func TestGatewayTags(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		input GatewayTags
+		want  GatewayTags
+		omit  bool
+	}{
+		{
+			input: GatewayTags{},
+			want:  GatewayTags{Tags: []string{}},
+			omit:  true,
+		},
+		{
+			input: GatewayTags{Enabled: true},
+			want:  GatewayTags{Enabled: true, Tags: []string{}},
+		},
+		{
+			input: GatewayTags{Enabled: true, Tags: []string{}},
+			want:  GatewayTags{Enabled: true, Tags: []string{}},
+		},
+		{
+			input: GatewayTags{Enabled: true, Tags: []string{"test"}},
+			want:  GatewayTags{Enabled: true, Tags: []string{"test"}},
+		},
+		{
+			input: GatewayTags{Enabled: true, Tags: []string{"t1", "t2"}},
+			want:  GatewayTags{Enabled: true, Tags: []string{"t1", "t2"}},
+		},
+		{
+			input: GatewayTags{Enabled: false, Tags: []string{"t1", "t2"}},
+			want:  GatewayTags{Enabled: false, Tags: []string{"t1", "t2"}},
+		},
+	}
+
+	t.Run("Fill GatewayTags from APIDef", func(t *testing.T) {
+		// We currently don't match APIDef direct fill with OAS
+		t.Skip() // TODO: TT-5720
+
+		t.Parallel()
+
+		for idx, tc := range testcases {
+			var api apidef.APIDefinition
+			tc.input.ExtractTo(&api)
+
+			got := new(GatewayTags)
+			got.Fill(api)
+
+			assert.Equal(t, tc.want, *got, fmt.Sprintf("Test case %d", idx))
+		}
+	})
+
+	t.Run("Fill OAS GatewayTags from APIDef", func(t *testing.T) {
+		t.Parallel()
+
+		for idx, tc := range testcases {
+			var api apidef.APIDefinition
+			tc.input.ExtractTo(&api)
+
+			var oas OAS
+			oas.Fill(api)
+
+			var schema = oas.GetTykExtension()
+			var got = schema.Server.GatewayTags
+
+			if tc.omit {
+				assert.Nil(t, got, idx)
+			} else {
+				assert.Equal(t, tc.want, *got, fmt.Sprintf("Test case %d", idx))
+			}
+		}
+	})
+}
+
 func TestClientCertificates(t *testing.T) {
 	t.Parallel()
 
@@ -65,42 +139,6 @@ func TestPinnedPublicKeys(t *testing.T) {
 	assert.Equal(t, pinnedPublicKeys, resultPinnedPublicKeys)
 }
 
-func TestTagsImport(t *testing.T) {
-	t.Parallel()
-
-	testcases := []struct {
-		title          string
-		input          *GatewayTags
-		expectDisabled bool
-		expectValues   []string
-	}{
-		{
-			"keep segment tags values if disabled",
-			&GatewayTags{Enabled: false, Tags: []string{"a", "b", "c"}},
-			true,
-			[]string{"a", "b", "c"},
-		},
-		{
-			"keep segment tags values if enabled",
-			&GatewayTags{Enabled: true, Tags: []string{"a", "b", "c"}},
-			false,
-			[]string{"a", "b", "c"},
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.title, func(t *testing.T) {
-			t.Parallel()
-
-			var apidef apidef.APIDefinition
-
-			tc.input.ExtractTo(&apidef)
-
-			assert.Equal(t, tc.expectDisabled, apidef.TagsDisabled)
-			assert.Equal(t, tc.expectValues, apidef.Tags)
-		})
-	}
-}
 func TestCustomDomain(t *testing.T) {
 	t.Run("extractTo api definition", func(t *testing.T) {
 		testcases := []struct {

apidef/oas/validator_test.go

@@ -71,6 +71,7 @@ func TestValidateOASObject(t *testing.T) {
 	invalidOASObject := validOASObject
 	invalidXTykAPIGateway := validXTykAPIGateway
 	invalidXTykAPIGateway.Info = Info{}
+	invalidXTykAPIGateway.Server.GatewayTags = &GatewayTags{Enabled: true, Tags: []string{}}
 	invalidOASObject.SetTykExtension(&invalidXTykAPIGateway)
 	invalidOASObject.Paths["/pets"].Get.Responses["200"].Value.Description = nil
 	invalidOAS3Definition, _ := invalidOASObject.MarshalJSON()

gateway/api_test.go

@@ -2038,10 +2038,6 @@ func TestOAS(t *testing.T) {
 				Value: "/oas-api/",
 				Strip: false,
 			},
-			GatewayTags: &oas.GatewayTags{
-				Enabled: false,
-				Tags:    []string{},
-			},
 		},
 	}
 
@@ -2161,6 +2157,7 @@ func TestOAS(t *testing.T) {
 				}
 
 				oldAPIInOAS.GetTykExtension().Info.Versioning = nil
+				oldAPIInOAS.GetTykExtension().Server.GatewayTags = nil
 
 				oldAPIInOAS.Paths = make(openapi3.Paths)
 				updatePath := "/tyk/apis/oas/" + apiID
@@ -2228,41 +2225,6 @@ func TestOAS(t *testing.T) {
 				testUpdateAPI(t, ts, &oasAPI, apiID, true)
 			})
 
-			t.Run("with oas and gateway tags disabled", func(t *testing.T) {
-				oasAPIInOAS := testGetOASAPI(t, ts, apiID, "oas api", "oas doc")
-
-				oasAPIInOAS.Extensions[oas.ExtensionTykAPIGateway] = oas.XTykAPIGateway{
-					Info: oas.Info{Name: "oas-updated oas api", ID: apiID},
-					Server: oas.Server{
-						ListenPath: oas.ListenPath{
-							Value: "/oas-updated",
-						},
-						GatewayTags: &oas.GatewayTags{
-							Enabled: false,
-							Tags:    []string{"rainbow"},
-						},
-					},
-				}
-
-				oasAPIInOAS.Paths = make(openapi3.Paths)
-
-				oasAPIInOAS.Info.Title = "oas-updated oas doc"
-				testUpdateAPI(t, ts, &oasAPIInOAS, apiID, true)
-
-				t.Run("get", func(t *testing.T) {
-					t.Run("in oas", func(t *testing.T) {
-						testGetOASAPI(t, ts, apiID, "oas-updated oas api", "oas-updated oas doc")
-					})
-
-					t.Run("in old", func(t *testing.T) {
-						testGetOldAPI(t, ts, apiID, "oas-updated oas api")
-					})
-				})
-
-				// Reset
-				testUpdateAPI(t, ts, &oasAPI, apiID, true)
-			})
-
 			t.Run("with oas", func(t *testing.T) {
 				oasAPIInOAS := testGetOASAPI(t, ts, apiID, "oas api", "oas doc")
 
@@ -2461,10 +2423,6 @@ func TestOAS(t *testing.T) {
 			fillPaths(&apiInOAS)
 			tykExt := apiInOAS.GetTykExtension()
 			tykExt.Info.Name = "patched-oas-api"
-			tykExt.Server.GatewayTags = &oas.GatewayTags{
-				Enabled: true,
-				Tags:    []string{},
-			}
 
 			apiInOAS.T.Info.Title = "patched-oas-doc"
 			testPatchOAS(t, ts, apiInOAS, nil, apiID)
@@ -2482,10 +2440,6 @@ func TestOAS(t *testing.T) {
 
 			tykExt := apiInOAS.GetTykExtension()
 			delete(apiInOAS.ExtensionProps.Extensions, oas.ExtensionTykAPIGateway)
-			tykExt.Server.GatewayTags = &oas.GatewayTags{
-				Enabled: true,
-				Tags:    []string{},
-			}
 
 			apiInOAS.T.Info.Title = "patched-oas-doc"
 			testPatchOAS(t, ts, apiInOAS, nil, apiID)
@@ -2522,10 +2476,6 @@ func TestOAS(t *testing.T) {
 				Enabled: true,
 				Name:    customDomain,
 			}
-			expectedTykExt.Server.GatewayTags = &oas.GatewayTags{
-				Enabled: true,
-				Tags:    []string{},
-			}
 
 			expectedTykExt.Middleware = &oas.Middleware{
 				Operations: oas.Operations{
@@ -2577,10 +2527,6 @@ func TestOAS(t *testing.T) {
 				Enabled: true,
 				Name:    customDomain,
 			}
-			expectedTykExt.Server.GatewayTags = &oas.GatewayTags{
-				Enabled: true,
-				Tags:    []string{},
-			}
 			expectedTykExt.Middleware = &oas.Middleware{
 				Operations: oas.Operations{
 					"petsGET": {
@@ -2772,6 +2718,7 @@ func TestOAS(t *testing.T) {
 				tykExt.Info.Name = "patched-oas-api"
 				tykExt.Info.Versioning.Default = "default"
 				tykExt.Info.Versioning.Versions = []oas.VersionToID{}
+				tykExt.Server.GatewayTags = nil
 				apiInOAS.T.Info = &openapi3.Info{Title: "patched-oas-doc"}
 				apiInOAS.OpenAPI = "3.0.3"