Skip to content
This repository was archived by the owner on Oct 10, 2023. It is now read-only.

Commit 98ceae9

Browse files
author
Anuj Chaudhari
committed
Refactor and fix populate default standalone discovery logic
1 parent aaa8a05 commit 98ceae9

File tree

10 files changed

+297
-110
lines changed

10 files changed

+297
-110
lines changed

‎pkg/v1/cli/command/core/discovery_source.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli"
1515
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli/common"
1616
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli/component"
17+
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli/discovery"
1718
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/config"
1819
)
1920

@@ -227,15 +228,6 @@ func createOCIDiscoverySource(discoveryName, uri string) *configv1alpha1.OCIDisc
227228
}
228229
}
229230

230-
// checkDiscoveryName returns true if discovery name exists else return false
231-
func checkDiscoveryName(ds configv1alpha1.PluginDiscovery, dn string) bool {
232-
return (ds.GCP != nil && ds.GCP.Name == dn) ||
233-
(ds.Kubernetes != nil && ds.Kubernetes.Name == dn) ||
234-
(ds.Local != nil && ds.Local.Name == dn) ||
235-
(ds.REST != nil && ds.REST.Name == dn) ||
236-
(ds.OCI != nil && ds.OCI.Name == dn)
237-
}
238-
239231
func discoverySourceNameAndType(ds configv1alpha1.PluginDiscovery) (string, string) {
240232
switch {
241233
case ds.GCP != nil:
@@ -255,7 +247,7 @@ func discoverySourceNameAndType(ds configv1alpha1.PluginDiscovery) (string, stri
255247

256248
func addDiscoverySource(discoverySources []configv1alpha1.PluginDiscovery, dsName, dsType, uri string) ([]configv1alpha1.PluginDiscovery, error) {
257249
for _, ds := range discoverySources {
258-
if checkDiscoveryName(ds, dsName) {
250+
if discovery.CheckDiscoveryName(ds, dsName) {
259251
return nil, fmt.Errorf("discovery name %q already exists", dsName)
260252
}
261253
}
@@ -273,7 +265,7 @@ func deleteDiscoverySource(discoverySources []configv1alpha1.PluginDiscovery, di
273265
newDiscoverySources := []configv1alpha1.PluginDiscovery{}
274266
found := false
275267
for _, ds := range discoverySources {
276-
if checkDiscoveryName(ds, discoveryName) {
268+
if discovery.CheckDiscoveryName(ds, discoveryName) {
277269
found = true
278270
continue
279271
}
@@ -291,7 +283,7 @@ func updateDiscoverySources(discoverySources []configv1alpha1.PluginDiscovery, d
291283

292284
found := false
293285
for _, ds := range discoverySources {
294-
if checkDiscoveryName(ds, dsName) {
286+
if discovery.CheckDiscoveryName(ds, dsName) {
295287
found = true
296288
ds, err = createDiscoverySource(dsType, dsName, uri)
297289
if err != nil {

‎pkg/v1/cli/common/defaults.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ var (
2929
IsContextAwareDiscoveryEnabled = ""
3030
)
3131

32+
// ContextAwareDiscoveryEnabled returns true if the IsContextAwareDiscoveryEnabled
33+
// is set to true during build time
3234
func ContextAwareDiscoveryEnabled() bool {
3335
return strings.EqualFold(IsContextAwareDiscoveryEnabled, "true")
3436
}

‎pkg/v1/cli/discovery/utils.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright 2021 VMware, Inc. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package discovery
5+
6+
import (
7+
configv1alpha1 "github.com/vmware-tanzu/tanzu-framework/apis/config/v1alpha1"
8+
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli/common"
9+
)
10+
11+
// CheckDiscoveryName returns true if discovery name exists else return false
12+
func CheckDiscoveryName(ds configv1alpha1.PluginDiscovery, dn string) bool {
13+
return (ds.GCP != nil && ds.GCP.Name == dn) ||
14+
(ds.Kubernetes != nil && ds.Kubernetes.Name == dn) ||
15+
(ds.Local != nil && ds.Local.Name == dn) ||
16+
(ds.REST != nil && ds.REST.Name == dn) ||
17+
(ds.OCI != nil && ds.OCI.Name == dn)
18+
}
19+
20+
// CompareDiscoverySource returns true if both discovery source are same for the given type
21+
func CompareDiscoverySource(ds1, ds2 configv1alpha1.PluginDiscovery, dsType string) bool {
22+
switch dsType {
23+
case common.DiscoveryTypeLocal:
24+
return compareLocalDiscoverySources(ds1, ds2)
25+
26+
case common.DiscoveryTypeOCI:
27+
return compareOCIDiscoverySources(ds1, ds2)
28+
29+
case common.DiscoveryTypeKubernetes:
30+
return compareK8sDiscoverySources(ds1, ds2)
31+
32+
case common.DiscoveryTypeGCP:
33+
return compareGCPDiscoverySources(ds1, ds2)
34+
35+
case common.DiscoveryTypeREST:
36+
return compareRESTDiscoverySources(ds1, ds2)
37+
}
38+
return false
39+
}
40+
41+
func compareGCPDiscoverySources(ds1, ds2 configv1alpha1.PluginDiscovery) bool {
42+
return ds1.GCP != nil && ds2.GCP != nil &&
43+
ds1.GCP.Name == ds2.GCP.Name &&
44+
ds1.GCP.Bucket == ds2.GCP.Bucket &&
45+
ds1.GCP.ManifestPath == ds2.GCP.ManifestPath
46+
}
47+
48+
func compareLocalDiscoverySources(ds1, ds2 configv1alpha1.PluginDiscovery) bool {
49+
return ds1.Local != nil && ds2.Local != nil &&
50+
ds1.Local.Name == ds2.Local.Name &&
51+
ds1.Local.Path == ds2.Local.Path
52+
}
53+
54+
func compareOCIDiscoverySources(ds1, ds2 configv1alpha1.PluginDiscovery) bool {
55+
return ds1.OCI != nil && ds2.OCI != nil &&
56+
ds1.OCI.Name == ds2.OCI.Name &&
57+
ds1.OCI.Image == ds2.OCI.Image
58+
}
59+
60+
func compareK8sDiscoverySources(ds1, ds2 configv1alpha1.PluginDiscovery) bool {
61+
return ds1.Kubernetes != nil && ds2.Kubernetes != nil &&
62+
ds1.Kubernetes.Name == ds2.Kubernetes.Name &&
63+
ds1.Kubernetes.Path == ds2.Kubernetes.Path &&
64+
ds1.Kubernetes.Context == ds2.Kubernetes.Context
65+
}
66+
67+
func compareRESTDiscoverySources(ds1, ds2 configv1alpha1.PluginDiscovery) bool {
68+
return ds1.REST != nil && ds2.REST != nil &&
69+
ds1.REST.Name == ds2.REST.Name &&
70+
ds1.REST.BasePath == ds2.REST.BasePath &&
71+
ds1.REST.Endpoint == ds2.REST.Endpoint
72+
}

‎pkg/v1/cli/discovery/utils_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2021 VMware, Inc. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package discovery
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
11+
configv1alpha1 "github.com/vmware-tanzu/tanzu-framework/apis/config/v1alpha1"
12+
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/cli/common"
13+
)
14+
15+
func Test_CheckDiscoveryName(t *testing.T) {
16+
assert := assert.New(t)
17+
18+
gcpDiscovery := configv1alpha1.PluginDiscovery{GCP: &configv1alpha1.GCPDiscovery{Name: "gcp-test"}}
19+
result := CheckDiscoveryName(gcpDiscovery, "gcp-test")
20+
assert.True(result)
21+
result = CheckDiscoveryName(gcpDiscovery, "test")
22+
assert.False(result)
23+
24+
ociDiscovery := configv1alpha1.PluginDiscovery{OCI: &configv1alpha1.OCIDiscovery{Name: "oci-test"}}
25+
result = CheckDiscoveryName(ociDiscovery, "oci-test")
26+
assert.True(result)
27+
result = CheckDiscoveryName(ociDiscovery, "test")
28+
assert.False(result)
29+
30+
k8sDiscovery := configv1alpha1.PluginDiscovery{Kubernetes: &configv1alpha1.KubernetesDiscovery{Name: "k8s-test"}}
31+
result = CheckDiscoveryName(k8sDiscovery, "k8s-test")
32+
assert.True(result)
33+
result = CheckDiscoveryName(k8sDiscovery, "test")
34+
assert.False(result)
35+
36+
localDiscovery := configv1alpha1.PluginDiscovery{Local: &configv1alpha1.LocalDiscovery{Name: "local-test"}}
37+
result = CheckDiscoveryName(localDiscovery, "local-test")
38+
assert.True(result)
39+
result = CheckDiscoveryName(localDiscovery, "test")
40+
assert.False(result)
41+
42+
restDiscovery := configv1alpha1.PluginDiscovery{REST: &configv1alpha1.GenericRESTDiscovery{Name: "rest-test"}}
43+
result = CheckDiscoveryName(restDiscovery, "rest-test")
44+
assert.True(result)
45+
result = CheckDiscoveryName(restDiscovery, "test")
46+
assert.False(result)
47+
}
48+
49+
func Test_CompareDiscoverySource(t *testing.T) {
50+
assert := assert.New(t)
51+
52+
ds1 := configv1alpha1.PluginDiscovery{GCP: &configv1alpha1.GCPDiscovery{Name: "gcp-test", Bucket: "bucket1", ManifestPath: "manifest1"}}
53+
ds2 := configv1alpha1.PluginDiscovery{GCP: &configv1alpha1.GCPDiscovery{Name: "gcp-test", Bucket: "bucket1", ManifestPath: "manifest1"}}
54+
result := CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeGCP)
55+
assert.True(result)
56+
ds2 = configv1alpha1.PluginDiscovery{GCP: &configv1alpha1.GCPDiscovery{Name: "gcp-test", Bucket: "bucket2", ManifestPath: "manifest1"}}
57+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeGCP)
58+
assert.False(result)
59+
60+
ds1 = configv1alpha1.PluginDiscovery{Local: &configv1alpha1.LocalDiscovery{Name: "gcp-test", Path: "path1"}}
61+
ds2 = configv1alpha1.PluginDiscovery{Local: &configv1alpha1.LocalDiscovery{Name: "gcp-test", Path: "path1"}}
62+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeLocal)
63+
assert.True(result)
64+
ds2 = configv1alpha1.PluginDiscovery{Local: &configv1alpha1.LocalDiscovery{Name: "gcp-test", Path: "path2"}}
65+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeLocal)
66+
assert.False(result)
67+
68+
ds1 = configv1alpha1.PluginDiscovery{OCI: &configv1alpha1.OCIDiscovery{Name: "gcp-test", Image: "image1"}}
69+
ds2 = configv1alpha1.PluginDiscovery{OCI: &configv1alpha1.OCIDiscovery{Name: "gcp-test", Image: "image1"}}
70+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeOCI)
71+
assert.True(result)
72+
ds2 = configv1alpha1.PluginDiscovery{OCI: &configv1alpha1.OCIDiscovery{Name: "gcp-test", Image: "image2"}}
73+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeOCI)
74+
assert.False(result)
75+
76+
ds1 = configv1alpha1.PluginDiscovery{OCI: &configv1alpha1.OCIDiscovery{Name: "gcp-test", Image: "image1"}}
77+
ds2 = configv1alpha1.PluginDiscovery{Local: &configv1alpha1.LocalDiscovery{Name: "gcp-test", Path: "path1"}}
78+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeOCI)
79+
assert.False(result)
80+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeLocal)
81+
assert.False(result)
82+
result = CompareDiscoverySource(ds1, ds2, common.DiscoveryTypeREST)
83+
assert.False(result)
84+
}

‎pkg/v1/cli/pluginmanager/manager.go‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -152,29 +152,7 @@ func AvailablePlugins(serverName string) ([]plugin.Discovered, error) {
152152
return nil, err
153153
}
154154

155-
availablePlugins := discoveredServerPlugins
156-
157-
for i := range discoveredStandalonePlugins {
158-
matchIndex := -1
159-
for j := range availablePlugins {
160-
if discoveredStandalonePlugins[i].Name == availablePlugins[j].Name {
161-
matchIndex = j
162-
break
163-
}
164-
}
165-
166-
// Add the standalone plugin to available plugin if it doesn't exist in the serverPlugins list
167-
// OR
168-
// Current standalone discovery type is local
169-
// We are overriding the discovered plugins that we got from server in case of 'local' discovery type
170-
// to allow developers to use the plugins that are built locally and not returned from the server
171-
// This local discovery is only used for development purpose and should not be used for production
172-
if config.DefaultStandaloneDiscoveryType == "local" && matchIndex >= 0 {
173-
availablePlugins[matchIndex] = discoveredStandalonePlugins[i]
174-
} else if matchIndex < 0 {
175-
availablePlugins = append(availablePlugins, discoveredStandalonePlugins[i])
176-
}
177-
}
155+
availablePlugins := availablePluginsFromStandaloneAndServerPlugins(discoveredServerPlugins, discoveredStandalonePlugins)
178156

179157
for i := range installedSeverPluginDesc {
180158
for j := range availablePlugins {
@@ -198,6 +176,33 @@ func AvailablePlugins(serverName string) ([]plugin.Discovered, error) {
198176
return availablePlugins, nil
199177
}
200178

179+
func availablePluginsFromStandaloneAndServerPlugins(discoveredServerPlugins, discoveredStandalonePlugins []plugin.Discovered) []plugin.Discovered {
180+
availablePlugins := discoveredServerPlugins
181+
182+
for i := range discoveredStandalonePlugins {
183+
matchIndex := -1
184+
for j := range availablePlugins {
185+
if discoveredStandalonePlugins[i].Name == availablePlugins[j].Name {
186+
matchIndex = j
187+
break
188+
}
189+
}
190+
191+
// Add the standalone plugin to available plugin if it doesn't exist in the serverPlugins list
192+
// OR
193+
// Current standalone discovery type is local and there is
194+
// We are overriding the discovered plugins that we got from server in case of 'local' discovery type
195+
// to allow developers to use the plugins that are built locally and not returned from the server
196+
// This local discovery is only used for development purpose and should not be used for production
197+
if config.GetDefaultStandaloneDiscoveryType() == common.DiscoveryTypeLocal && matchIndex >= 0 {
198+
availablePlugins[matchIndex] = discoveredStandalonePlugins[i]
199+
} else if matchIndex < 0 {
200+
availablePlugins = append(availablePlugins, discoveredStandalonePlugins[i])
201+
}
202+
}
203+
return availablePlugins
204+
}
205+
201206
// InstalledPlugins returns the installed plugins.
202207
// If serverName is empty(""), return only installed standalone plugins
203208
func InstalledPlugins(serverName string, exclude ...string) (serverPlugins, standalonePlugins []cliv1alpha1.PluginDescriptor, err error) {

‎pkg/v1/config/clientconfig.go‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -152,75 +152,6 @@ func NewClientConfig() (*configv1alpha1.ClientConfig, error) {
152152
return c, nil
153153
}
154154

155-
func populateDefaultStandaloneDiscovery(c *configv1alpha1.ClientConfig) bool {
156-
if c.ClientOptions == nil {
157-
c.ClientOptions = &configv1alpha1.ClientOptions{}
158-
}
159-
if c.ClientOptions.CLI == nil {
160-
c.ClientOptions.CLI = &configv1alpha1.CLIOptions{}
161-
}
162-
if c.ClientOptions.CLI.DiscoverySources == nil {
163-
c.ClientOptions.CLI.DiscoverySources = make([]configv1alpha1.PluginDiscovery, 0)
164-
}
165-
166-
switch DefaultStandaloneDiscoveryType {
167-
case common.DiscoveryTypeOCI:
168-
return populateDefaultStandaloneDiscoveryOCI(c)
169-
case common.DiscoveryTypeLocal:
170-
return populateDefaultStandaloneDiscoveryLocal(c)
171-
default:
172-
log.Warning("unsupported default standalone discovery configuration")
173-
}
174-
return false
175-
}
176-
177-
func populateDefaultStandaloneDiscoveryLocal(c *configv1alpha1.ClientConfig) bool {
178-
for _, ds := range c.ClientOptions.CLI.DiscoverySources {
179-
if ds.Local != nil && ds.Local.Name == DefaultStandaloneDiscoveryName {
180-
if ds.Local.Path == DefaultStandaloneDiscoveryLocalPath {
181-
return false
182-
}
183-
ds.Local.Path = DefaultStandaloneDiscoveryLocalPath
184-
return true
185-
}
186-
}
187-
188-
defaultDiscovery := configv1alpha1.PluginDiscovery{
189-
Local: &configv1alpha1.LocalDiscovery{
190-
Name: DefaultStandaloneDiscoveryName,
191-
Path: DefaultStandaloneDiscoveryLocalPath,
192-
},
193-
}
194-
195-
// Prepend default discovery to available discovery sources
196-
c.ClientOptions.CLI.DiscoverySources = append([]configv1alpha1.PluginDiscovery{defaultDiscovery}, c.ClientOptions.CLI.DiscoverySources...)
197-
return true
198-
}
199-
200-
func populateDefaultStandaloneDiscoveryOCI(c *configv1alpha1.ClientConfig) bool {
201-
defaultStandaloneDiscoveryImage := DefaultStandaloneDiscoveryImage()
202-
for _, ds := range c.ClientOptions.CLI.DiscoverySources {
203-
if ds.OCI != nil && ds.OCI.Name == DefaultStandaloneDiscoveryName {
204-
if ds.OCI.Image == defaultStandaloneDiscoveryImage {
205-
return false
206-
}
207-
ds.OCI.Image = defaultStandaloneDiscoveryImage
208-
return true
209-
}
210-
}
211-
212-
defaultDiscovery := configv1alpha1.PluginDiscovery{
213-
OCI: &configv1alpha1.OCIDiscovery{
214-
Name: DefaultStandaloneDiscoveryName,
215-
Image: defaultStandaloneDiscoveryImage,
216-
},
217-
}
218-
219-
// Prepend default discovery to available discovery sources
220-
c.ClientOptions.CLI.DiscoverySources = append([]configv1alpha1.PluginDiscovery{defaultDiscovery}, c.ClientOptions.CLI.DiscoverySources...)
221-
return true
222-
}
223-
224155
func populateDefaultCliFeatureValues(c *configv1alpha1.ClientConfig, defaultCliFeatureFlags map[string]bool) error {
225156
for featureName, flagValue := range defaultCliFeatureFlags {
226157
plugin, flag, err := c.SplitFeaturePath(featureName)

‎pkg/v1/config/clientconfig_test.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ func TestConfigPopulateDefaultStandaloneDiscovery(t *testing.T) {
382382
assert.Equal(cfg.ClientOptions.CLI.DiscoverySources[0].OCI.Image, "fake.image.repo/package/standalone-plugins:v1.0.0")
383383
}
384384

385-
func TestConfigPopulateDefaultStandaloneDiscoveryWhenPresentAndImageIsSame(t *testing.T) {
385+
func TestConfigPopulateDefaultStandaloneDiscoveryWhenDefaultDiscoveryExistsAndIsSame(t *testing.T) {
386386
cfg := &configv1alpha1.ClientConfig{
387387
ClientOptions: &configv1alpha1.ClientOptions{
388388
CLI: &configv1alpha1.CLIOptions{
@@ -408,7 +408,7 @@ func TestConfigPopulateDefaultStandaloneDiscoveryWhenPresentAndImageIsSame(t *te
408408
assert.Equal(cfg.ClientOptions.CLI.DiscoverySources[0].OCI.Image, "fake.image.repo/package/standalone-plugins:v1.0.0")
409409
}
410410

411-
func TestConfigPopulateDefaultStandaloneDiscoveryWhenPresentAndImageIsNotSame(t *testing.T) {
411+
func TestConfigPopulateDefaultStandaloneDiscoveryWhenDefaultDiscoveryExistsAndIsNotSame(t *testing.T) {
412412
cfg := &configv1alpha1.ClientConfig{
413413
ClientOptions: &configv1alpha1.ClientOptions{
414414
CLI: &configv1alpha1.CLIOptions{
@@ -450,8 +450,8 @@ func TestConfigPopulateDefaultStandaloneDiscoveryLocal(t *testing.T) {
450450
},
451451
},
452452
}
453-
DefaultStandaloneDiscoveryType = "local"
454-
DefaultStandaloneDiscoveryLocalPath = "local/path"
453+
454+
configureTestDefaultStandaloneDiscoveryLocal()
455455

456456
assert := assert.New(t)
457457

@@ -524,3 +524,8 @@ func configureTestDefaultStandaloneDiscoveryOCI() {
524524
DefaultStandaloneDiscoveryImagePath = "package/standalone-plugins"
525525
DefaultStandaloneDiscoveryImageTag = "v1.0.0"
526526
}
527+
528+
func configureTestDefaultStandaloneDiscoveryLocal() {
529+
DefaultStandaloneDiscoveryType = "local"
530+
DefaultStandaloneDiscoveryLocalPath = "local/path"
531+
}

0 commit comments

Comments
 (0)