From ad6fa166179cad8306dc2e939939f0ae64c5f3f0 Mon Sep 17 00:00:00 2001 From: Tao Zou Date: Mon, 25 Nov 2024 10:49:50 +0800 Subject: [PATCH] Add AllowOverwrite for some clients Some clients may operate resources created by other PI so 'X-Allow-Overwrite' should be True. Add the header for those clients. Remove headerconfig.go since it's only used for deprecated rest connector Test Done: 1. use user=admin to run cleanup 2. check if subnetport has been deleted which created by WCP user --- .../networkpolicy_controller_test.go | 2 +- .../securitypolicy_controller_test.go | 2 +- pkg/nsx/client.go | 11 ++-- pkg/nsx/client_test.go | 37 +++++++++++ pkg/nsx/cluster.go | 18 ++++-- pkg/nsx/cluster_test.go | 2 +- pkg/nsx/headerconfig.go | 64 ------------------- pkg/nsx/headerconfig_test.go | 53 --------------- pkg/nsx/services/common/store_test.go | 2 +- .../ipaddressallocation/builder_test.go | 2 +- .../ipaddressallocation_test.go | 2 +- pkg/nsx/services/securitypolicy/store_test.go | 6 +- pkg/nsx/services/securitypolicy/wrap_test.go | 2 +- .../services/staticroute/staticroute_test.go | 2 +- pkg/nsx/services/subnet/store_test.go | 2 +- pkg/nsx/services/subnet/wrap_test.go | 2 +- pkg/nsx/services/vpc/store_test.go | 2 +- pkg/nsx/services/vpc/vpc_test.go | 2 +- 18 files changed, 73 insertions(+), 140 deletions(-) delete mode 100644 pkg/nsx/headerconfig.go delete mode 100644 pkg/nsx/headerconfig_test.go diff --git a/pkg/controllers/networkpolicy/networkpolicy_controller_test.go b/pkg/controllers/networkpolicy/networkpolicy_controller_test.go index 3e2bd0992..60a3a0657 100644 --- a/pkg/controllers/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controllers/networkpolicy/networkpolicy_controller_test.go @@ -81,7 +81,7 @@ func (m *MockManager) Start(context.Context) error { func fakeService() *securitypolicy.SecurityPolicyService { c := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(c) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := &securitypolicy.SecurityPolicyService{ Service: common.Service{ diff --git a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go index 86ad9140b..62cdce218 100644 --- a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go +++ b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go @@ -51,7 +51,7 @@ import ( func fakeService() *securitypolicy.SecurityPolicyService { c := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(c) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := &securitypolicy.SecurityPolicyService{ Service: common.Service{ diff --git a/pkg/nsx/client.go b/pkg/nsx/client.go index e64bbb423..69a9462b7 100644 --- a/pkg/nsx/client.go +++ b/pkg/nsx/client.go @@ -136,8 +136,11 @@ func (ck *NSXHealthChecker) CheckNSXHealth(req *http.Request) error { } func restConnector(c *Cluster) client.Connector { - connector, _ := c.NewRestConnector() - return connector + return c.NewRestConnector() +} + +func restConnectorAllowOverwrite(c *Cluster) client.Connector { + return c.NewRestConnectorAllowOverwrite() } func GetClient(cf *config.NSXOperatorConfig) *Client { @@ -177,13 +180,13 @@ func GetClient(cf *config.NSXOperatorConfig) *Client { staticRouteClient := vpcs.NewStaticRoutesClient(restConnector(cluster)) natRulesClient := nat.NewNatRulesClient(restConnector(cluster)) vpcGroupClient := vpcs.NewGroupsClient(restConnector(cluster)) - portClient := subnets.NewPortsClient(restConnector(cluster)) + portClient := subnets.NewPortsClient(restConnectorAllowOverwrite(cluster)) portStateClient := ports.NewStateClient(restConnector(cluster)) ipPoolClient := subnets.NewIpPoolsClient(restConnector(cluster)) ipAllocationClient := ip_pools.NewIpAllocationsClient(restConnector(cluster)) subnetsClient := vpcs.NewSubnetsClient(restConnector(cluster)) subnetStatusClient := subnets.NewStatusClient(restConnector(cluster)) - ipAddressAllocationClient := vpcs.NewIpAddressAllocationsClient(restConnector(cluster)) + ipAddressAllocationClient := vpcs.NewIpAddressAllocationsClient(restConnectorAllowOverwrite(cluster)) vpcLBSClient := vpcs.NewVpcLbsClient(restConnector(cluster)) vpcLbVirtualServersClient := vpcs.NewVpcLbVirtualServersClient(restConnector(cluster)) vpcLbPoolsClient := vpcs.NewVpcLbPoolsClient(restConnector(cluster)) diff --git a/pkg/nsx/client_test.go b/pkg/nsx/client_test.go index c305d8614..be7ae0cda 100644 --- a/pkg/nsx/client_test.go +++ b/pkg/nsx/client_test.go @@ -6,6 +6,7 @@ package nsx import ( "fmt" "net/http" + "net/http/httptest" "reflect" "strings" "testing" @@ -13,6 +14,7 @@ import ( "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/search" "github.com/vmware-tanzu/nsx-operator/pkg/config" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" @@ -172,3 +174,38 @@ func TestSRGetClient(t *testing.T) { fmt.Printf("b is %v \n", b[2]) } +func TestRestConnectorAllowOverwrite(t *testing.T) { + resVersion := `{ + "node_version": "3.1.3.3.0.18844962", + "product_version": "3.1.3.3.0.18844959" + }` + resHealth := `{ + "healthy" : true, + "components_health" : "MANAGER:UP, SEARCH:UP, UI:UP, NODE_MGMT:UP" + }` + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("header %v", r.Header) + w.WriteHeader(http.StatusOK) + if strings.Contains(r.URL.Path, "search") { + allowOverwrite, _ := r.Header["X-Allow-Overwrite"] + assert.Equal(t, "True", allowOverwrite[0]) + } + if strings.Contains(r.URL.Path, "reverse-proxy/node/health") { + w.Write(([]byte(resHealth))) + } else { + w.Write([]byte(resVersion)) + } + })) + defer ts.Close() + thumbprint := []string{"123"} + index := strings.Index(ts.URL, "//") + a := ts.URL[index+2:] + config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint) + cluster, _ := NewCluster(config) + nsxVersion, err := cluster.GetVersion() + assert.Equal(t, err, nil) + assert.Equal(t, nsxVersion.NodeVersion, "3.1.3.3.0.18844962") + + client := search.NewQueryClient(cluster.NewRestConnectorAllowOverwrite()) + client.List("search", nil, nil, nil, nil, nil) +} diff --git a/pkg/nsx/cluster.go b/pkg/nsx/cluster.go index 438ac844e..286bb466f 100644 --- a/pkg/nsx/cluster.go +++ b/pkg/nsx/cluster.go @@ -162,13 +162,23 @@ func (cluster *Cluster) CreateServerUrl(host string, scheme string) string { } // NewRestConnector creates a RestConnector used for SDK client. -// HeaderConfig is used to use http header for request, it could be ignored if no extra header needed. -func (cluster *Cluster) NewRestConnector() (policyclient.Connector, *HeaderConfig) { +func (cluster *Cluster) NewRestConnector() policyclient.Connector { nsxtUrl := cluster.CreateServerUrl(cluster.endpoints[0].Host(), cluster.endpoints[0].Scheme()) connector := policyclient.NewConnector(nsxtUrl, policyclient.UsingRest(nil), policyclient.WithHttpClient(cluster.client)) - header := CreateHeaderConfig(false, false, cluster.config.AllowOverwriteHeader) connector.NewExecutionContext() - return connector, header + return connector +} +func SetAllowOverwriteHeader(req *http.Request) error { + // Set the header X-Allow-Overwrite to True + req.Header.Set("X-Allow-Overwrite", "True") + return nil +} +func (cluster *Cluster) NewRestConnectorAllowOverwrite() policyclient.Connector { + nsxtUrl := cluster.CreateServerUrl(cluster.endpoints[0].Host(), cluster.endpoints[0].Scheme()) + policyclient.WithRequestProcessors() + connector := policyclient.NewConnector(nsxtUrl, policyclient.UsingRest(nil), policyclient.WithHttpClient(cluster.client), policyclient.WithRequestProcessors(SetAllowOverwriteHeader)) + connector.NewExecutionContext() + return connector } func (cluster *Cluster) UsingEnvoy() bool { diff --git a/pkg/nsx/cluster_test.go b/pkg/nsx/cluster_test.go index 13970c90d..ec06eca4c 100644 --- a/pkg/nsx/cluster_test.go +++ b/pkg/nsx/cluster_test.go @@ -96,7 +96,7 @@ func TestCluster_NewRestConnector(t *testing.T) { thumbprint := []string{"123"} config := NewConfig(a, "admin", "passw0rd", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, thumbprint) c, _ := NewCluster(config) - con, _ := c.NewRestConnector() + con := c.NewRestConnector() assert.NotNil(t, con) } diff --git a/pkg/nsx/headerconfig.go b/pkg/nsx/headerconfig.go deleted file mode 100644 index 2079413b4..000000000 --- a/pkg/nsx/headerconfig.go +++ /dev/null @@ -1,64 +0,0 @@ -/* Copyright © 2021 VMware, Inc. All Rights Reserved. - SPDX-License-Identifier: Apache-2.0 */ - -package nsx - -import ( - "net/http" - - "github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client" -) - -// HeaderConfig updates http request header. -type HeaderConfig struct { - xAllowOverwrite bool - nsxEnablePartialPatch bool - // configXallowOverwrite comes from config, it's a global parameter. - configXallowOverwrite bool -} - -// CreateHeaderConfig creates HeaderConfig. -func CreateHeaderConfig(xAllowOverwrite bool, nsxEnablePartialPatch bool, configXallowOverwrite bool) *HeaderConfig { - header := &HeaderConfig{ - xAllowOverwrite: xAllowOverwrite, - nsxEnablePartialPatch: nsxEnablePartialPatch, - configXallowOverwrite: configXallowOverwrite, - } - return header -} - -// Process adds header to http.Request depending on configuration. -func (headerConfig *HeaderConfig) Process(req *http.Request) error { - if headerConfig.configXallowOverwrite { - if headerConfig.xAllowOverwrite { - req.Header["X-Allow-Overwrite"] = []string{"true"} - } - } - if headerConfig.nsxEnablePartialPatch { - req.Header.Set("nsx-enable-partial-patch", "true") - } - return nil -} - -// SetXAllowOverrite sets XAllowoverrite. -func (headerConfig *HeaderConfig) SetXAllowOverrite(value bool) *HeaderConfig { - headerConfig.xAllowOverwrite = value - return headerConfig -} - -// SetNSXEnablePartialPatch sets NSXEnablePartialPatch. -func (headerConfig *HeaderConfig) SetNSXEnablePartialPatch(value bool) *HeaderConfig { - headerConfig.nsxEnablePartialPatch = value - return headerConfig -} - -// SetConfigXallowOverwrite sets configXallowOverwrite. -func (headerConfig *HeaderConfig) SetConfigXallowOverwrite(value bool) *HeaderConfig { - headerConfig.configXallowOverwrite = value - return headerConfig -} - -// Done updates request process of RestConnector. -func (headerConfig *HeaderConfig) Done(connector *client.RestConnector) { - connector.AddRequestProcessor(headerConfig) -} diff --git a/pkg/nsx/headerconfig_test.go b/pkg/nsx/headerconfig_test.go deleted file mode 100644 index 779e4fa32..000000000 --- a/pkg/nsx/headerconfig_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package nsx - -import ( - "fmt" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - policyclient "github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client" - "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/infra" - "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" -) - -func TestHeaderConfig(t *testing.T) { - assert := assert.New(t) - partial := false - result := `{ - "healthy" : true, - "components_health" : "POLICY:UP, SEARCH:UP, MANAGER:UP, NODE_MGMT:UP, UI:UP" - }` - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Index(r.URL.Path, "tier-1s") > 0 { - if r.Header.Get("nsx-enable-partial-patch") == "true" { - partial = true - } - } - w.WriteHeader(http.StatusOK) - w.Write([]byte(result)) - - })) - defer ts.Close() - httpClient := http.Client{ - Transport: &http.Transport{}, - } - connector := policyclient.NewRestConnector(fmt.Sprintf("%s", ts.URL), httpClient) - header := CreateHeaderConfig(false, false, false) - header.SetNSXEnablePartialPatch(true).Done(connector) - client := infra.NewTier1sClient(connector) - tier1 := model.Tier1{} - client.Patch("test-tier1-id", tier1) - client.Patch("test-tier1-id", tier1) - assert.True(partial) -} - -func TestCreateHeaderConfig(t *testing.T) { - cfg := CreateHeaderConfig(true, true, true) - assert.NotNil(t, cfg) - cfg.SetXAllowOverrite(false) - cfg.SetConfigXallowOverwrite(false) - cfg.SetNSXEnablePartialPatch(false) -} diff --git a/pkg/nsx/services/common/store_test.go b/pkg/nsx/services/common/store_test.go index 8a769c040..bc6fe55df 100644 --- a/pkg/nsx/services/common/store_test.go +++ b/pkg/nsx/services/common/store_test.go @@ -141,7 +141,7 @@ var filterTag = func(v []model.Tag) []string { func Test_InitializeResourceStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := Service{ NSXClient: &nsx.Client{ diff --git a/pkg/nsx/services/ipaddressallocation/builder_test.go b/pkg/nsx/services/ipaddressallocation/builder_test.go index a117e1cae..20612007f 100644 --- a/pkg/nsx/services/ipaddressallocation/builder_test.go +++ b/pkg/nsx/services/ipaddressallocation/builder_test.go @@ -38,7 +38,7 @@ func createService(t *testing.T) (*vpc.VPCService, *gomock.Controller, *mocks.Mo config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() mockCtrl := gomock.NewController(t) mockVpcclient := mocks.NewMockVpcsClient(mockCtrl) diff --git a/pkg/nsx/services/ipaddressallocation/ipaddressallocation_test.go b/pkg/nsx/services/ipaddressallocation/ipaddressallocation_test.go index 64d8a8902..9be1c5dd2 100644 --- a/pkg/nsx/services/ipaddressallocation/ipaddressallocation_test.go +++ b/pkg/nsx/services/ipaddressallocation/ipaddressallocation_test.go @@ -32,7 +32,7 @@ func createIPAddressAllocationService(t *testing.T) (*IPAddressAllocationService config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() mockCtrl := gomock.NewController(t) mockVPCIPAddressAllocationclient := mocks.NewMockIPAddressAllocationClient(mockCtrl) diff --git a/pkg/nsx/services/securitypolicy/store_test.go b/pkg/nsx/services/securitypolicy/store_test.go index ef27c9ba8..bf2b16b3a 100644 --- a/pkg/nsx/services/securitypolicy/store_test.go +++ b/pkg/nsx/services/securitypolicy/store_test.go @@ -150,7 +150,7 @@ func Test_KeyFunc(t *testing.T) { func Test_InitializeRuleStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := SecurityPolicyService{ Service: common.Service{ @@ -201,7 +201,7 @@ func Test_InitializeRuleStore(t *testing.T) { func Test_InitializeGroupStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := SecurityPolicyService{ Service: common.Service{ @@ -252,7 +252,7 @@ func Test_InitializeGroupStore(t *testing.T) { func Test_InitializeSecurityPolicyStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := SecurityPolicyService{ Service: common.Service{ diff --git a/pkg/nsx/services/securitypolicy/wrap_test.go b/pkg/nsx/services/securitypolicy/wrap_test.go index 4cba7a071..98925cb0d 100644 --- a/pkg/nsx/services/securitypolicy/wrap_test.go +++ b/pkg/nsx/services/securitypolicy/wrap_test.go @@ -134,7 +134,7 @@ func (f fakeVPCGroupClient) Update(orgIdParam string, projectIdParam string, vpc func fakeSecurityPolicyService() *SecurityPolicyService { c := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(c) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() fakeService := &SecurityPolicyService{ Service: common.Service{ NSXClient: &nsx.Client{ diff --git a/pkg/nsx/services/staticroute/staticroute_test.go b/pkg/nsx/services/staticroute/staticroute_test.go index 8e6eee64b..bcfabd749 100644 --- a/pkg/nsx/services/staticroute/staticroute_test.go +++ b/pkg/nsx/services/staticroute/staticroute_test.go @@ -59,7 +59,7 @@ func createService(t *testing.T) (*StaticRouteService, *gomock.Controller, *mock config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() mockCtrl := gomock.NewController(t) mockStaticRouteclient := mocks.NewMockStaticRoutesClient(mockCtrl) diff --git a/pkg/nsx/services/subnet/store_test.go b/pkg/nsx/services/subnet/store_test.go index 57ccfe75c..556051567 100644 --- a/pkg/nsx/services/subnet/store_test.go +++ b/pkg/nsx/services/subnet/store_test.go @@ -65,7 +65,7 @@ func Test_KeyFunc(t *testing.T) { func Test_InitializeSubnetStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{ common.TagScopeSubnetCRUID: subnetIndexFunc, diff --git a/pkg/nsx/services/subnet/wrap_test.go b/pkg/nsx/services/subnet/wrap_test.go index e927989ff..45da5a0ab 100644 --- a/pkg/nsx/services/subnet/wrap_test.go +++ b/pkg/nsx/services/subnet/wrap_test.go @@ -17,7 +17,7 @@ import ( func fakeService() *SubnetService { c := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(c) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() service := &SubnetService{ Service: common.Service{ NSXClient: &nsx.Client{ diff --git a/pkg/nsx/services/vpc/store_test.go b/pkg/nsx/services/vpc/store_test.go index 4414dd54b..09f110bd3 100644 --- a/pkg/nsx/services/vpc/store_test.go +++ b/pkg/nsx/services/vpc/store_test.go @@ -70,7 +70,7 @@ func Test_filterTag(t *testing.T) { func Test_InitializeVPCStore(t *testing.T) { config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() vpcCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{}) vpcStore := &VPCStore{ResourceStore: common.ResourceStore{ Indexer: vpcCacheIndexer, diff --git a/pkg/nsx/services/vpc/vpc_test.go b/pkg/nsx/services/vpc/vpc_test.go index 09714219a..350e416ec 100644 --- a/pkg/nsx/services/vpc/vpc_test.go +++ b/pkg/nsx/services/vpc/vpc_test.go @@ -57,7 +57,7 @@ func createService(t *testing.T) (*VPCService, *gomock.Controller, *mocks.MockVp config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{}) cluster, _ := nsx.NewCluster(config2) - rc, _ := cluster.NewRestConnector() + rc := cluster.NewRestConnector() mockCtrl := gomock.NewController(t) mockVpcclient := mocks.NewMockVpcsClient(mockCtrl)