Repository: kumahq/kuma
Pull request #1673 (2021-03-11 11:52:01)

fix(*) make sure we enumerate all types in kumactl
### Summary

Re-check all the places where we enumerate all resource types. Fix tests

### Issues resolved

Fix https://github.com/kumahq/kuma/issues/1658

### Documentation

- [ ] Internal



Patch:
diff --git a/app/kumactl/pkg/cmd/root_context.go b/app/kumactl/pkg/cmd/root_context.go
index 9c694b0be..91584f86b 100644
--- a/app/kumactl/pkg/cmd/root_context.go
+++ b/app/kumactl/pkg/cmd/root_context.go
@@ -66,20 +66,21 @@ func DefaultRootContext() *RootContext {
 			NewAPIServerClient:         kumactl_resources.NewAPIServerClient,
 		},
 		TypeArgs: map[string]core_model.ResourceType{
-			"mesh":               core_mesh.MeshType,
+			"circuit-breaker":    core_mesh.CircuitBreakerType,
 			"dataplane":          core_mesh.DataplaneType,
-			"externalservice":    core_mesh.ExternalServiceType,
+			"external-service":   core_mesh.ExternalServiceType,
+			"fault-injection":    core_mesh.FaultInjectionType,
 			"healthcheck":        core_mesh.HealthCheckType,
+			"mesh":               core_mesh.MeshType,
 			"proxytemplate":      core_mesh.ProxyTemplateType,
+			"retry":              core_mesh.RetryType,
+			"timeout":            core_mesh.TimeoutType,
 			"traffic-log":        core_mesh.TrafficLogType,
 			"traffic-permission": core_mesh.TrafficPermissionType,
 			"traffic-route":      core_mesh.TrafficRouteType,
 			"traffic-trace":      core_mesh.TrafficTraceType,
-			"fault-injection":    core_mesh.FaultInjectionType,
-			"circuit-breaker":    core_mesh.CircuitBreakerType,
-			"retry":              core_mesh.RetryType,
-			"secret":             system.SecretType,
 			"global-secret":      system.GlobalSecretType,
+			"secret":             system.SecretType,
 			"zone":               system.ZoneType,
 		},
 		InstallCpContext:      install_context.DefaultInstallCpContext(),
@@ -91,10 +92,11 @@ func DefaultRootContext() *RootContext {
 func (rc *RootContext) TypeForArg(arg string) (core_model.ResourceType, error) {
 	typ, ok := rc.TypeArgs[arg]
 	if !ok {
-		return "", errors.Errorf("unknown TYPE: %s. Allowed values: mesh, dataplane, "+
-			"healthcheck, proxytemplate, traffic-log, traffic-permission, traffic-route, "+
-			"traffic-trace, fault-injection, circuit-breaker, retry, secret, zone",
-			arg)
+		allowedValues := ""
+		for v := range rc.TypeArgs {
+			allowedValues += v + ", "
+		}
+		return "", errors.Errorf("unknown TYPE: %s. Allowed values: %s:", arg, allowedValues)
 	}
 	return typ, nil
 }
diff --git a/pkg/kds/global/components.go b/pkg/kds/global/components.go
index 7828f6c1b..3ae9341ab 100644
--- a/pkg/kds/global/components.go
+++ b/pkg/kds/global/components.go
@@ -32,19 +32,19 @@ import (
 var (
 	kdsGlobalLog  = core.Log.WithName("kds-global")
 	ProvidedTypes = []model.ResourceType{
-		mesh.MeshType,
+		mesh.CircuitBreakerType,
 		mesh.DataplaneType,
 		mesh.ExternalServiceType,
-		mesh.CircuitBreakerType,
 		mesh.FaultInjectionType,
 		mesh.HealthCheckType,
+		mesh.MeshType,
+		mesh.ProxyTemplateType,
+		mesh.RetryType,
+		mesh.TimeoutType,
 		mesh.TrafficLogType,
 		mesh.TrafficPermissionType,
 		mesh.TrafficRouteType,
 		mesh.TrafficTraceType,
-		mesh.ProxyTemplateType,
-		mesh.RetryType,
-		mesh.TimeoutType,
 		system.SecretType,
 		system.ConfigType,
 	}
diff --git a/pkg/kds/remote/components.go b/pkg/kds/remote/components.go
index dbe2276fe..38d76a1da 100644
--- a/pkg/kds/remote/components.go
+++ b/pkg/kds/remote/components.go
@@ -28,19 +28,19 @@ var (
 		mesh.DataplaneInsightType,
 	}
 	ConsumedTypes = []model.ResourceType{
-		mesh.MeshType,
+		mesh.CircuitBreakerType,
 		mesh.DataplaneType,
 		mesh.ExternalServiceType,
-		mesh.CircuitBreakerType,
 		mesh.FaultInjectionType,
 		mesh.HealthCheckType,
+		mesh.MeshType,
+		mesh.ProxyTemplateType,
+		mesh.RetryType,
+		mesh.TimeoutType,
 		mesh.TrafficLogType,
 		mesh.TrafficPermissionType,
 		mesh.TrafficRouteType,
 		mesh.TrafficTraceType,
-		mesh.ProxyTemplateType,
-		mesh.RetryType,
-		mesh.TimeoutType,
 		system.SecretType,
 		system.ConfigType,
 	}
diff --git a/pkg/kds/types.go b/pkg/kds/types.go
index 617716a7d..d30f55aeb 100644
--- a/pkg/kds/types.go
+++ b/pkg/kds/types.go
@@ -14,19 +14,20 @@ const (
 
 var (
 	SupportedTypes = []model.ResourceType{
-		mesh.MeshType,
+		mesh.CircuitBreakerType,
 		mesh.DataplaneType,
 		mesh.DataplaneInsightType,
 		mesh.ExternalServiceType,
-		mesh.CircuitBreakerType,
 		mesh.FaultInjectionType,
 		mesh.HealthCheckType,
+		mesh.MeshType,
+		mesh.ProxyTemplateType,
+		mesh.RetryType,
+		mesh.TimeoutType,
 		mesh.TrafficLogType,
 		mesh.TrafficPermissionType,
 		mesh.TrafficRouteType,
 		mesh.TrafficTraceType,
-		mesh.ProxyTemplateType,
-		mesh.RetryType,
 		system.SecretType,
 		system.ConfigType,
 	}


Test patch:
diff --git a/app/kumactl/cmd/delete/delete_test.go b/app/kumactl/cmd/delete/delete_test.go
index 6560c1cb5..01b9e101f 100644
--- a/app/kumactl/cmd/delete/delete_test.go
+++ b/app/kumactl/cmd/delete/delete_test.go
@@ -75,15 +75,9 @@ var _ = Describe("kumactl delete ", func() {
 			// then
 			Expect(err).To(HaveOccurred())
 			// and
-			Expect(err.Error()).To(Equal("unknown TYPE: some-type. Allowed values: mesh, " +
-				"dataplane, healthcheck, proxytemplate, traffic-log, traffic-permission, " +
-				"traffic-route, traffic-trace, fault-injection, circuit-breaker, retry, secret, " +
-				"zone"))
+			Expect(err.Error()).To(ContainSubstring("unknown TYPE: some-type. Allowed values:"))
 			// and
-			Expect(outbuf.String()).To(MatchRegexp("unknown TYPE: some-type. " +
-				"Allowed values: mesh, dataplane, healthcheck, proxytemplate, traffic-log, " +
-				"traffic-permission, traffic-route, traffic-trace, fault-injection, " +
-				"circuit-breaker, retry, secret, zone"))
+			Expect(outbuf.String()).To(ContainSubstring("unknown TYPE: some-type. Allowed values:"))
 			// and
 			Expect(errbuf.Bytes()).To(BeEmpty())
 		})
@@ -144,29 +138,53 @@ var _ = Describe("kumactl delete ", func() {
 					// then
 					Expect(err).ToNot(HaveOccurred())
 				},
+				Entry("circuit-breaker", testCase{
+					typ:             "circuit-breaker",
+					name:            "web",
+					resource:        func() core_model.Resource { return mesh_core.NewCircuitBreakerResource() },
+					expectedMessage: "deleted CircuitBreaker \"web\"\n",
+				}),
 				Entry("dataplanes", testCase{
 					typ:             "dataplane",
 					name:            "web",
 					resource:        func() core_model.Resource { return mesh_core.NewDataplaneResource() },
 					expectedMessage: "deleted Dataplane \"web\"\n",
 				}),
+				Entry("external-services", testCase{
+					typ:             "external-service",
+					name:            "httpbin",
+					resource:        func() core_model.Resource { return mesh_core.NewExternalServiceResource() },
+					expectedMessage: "deleted ExternalService \"httpbin\"\n",
+				}),
+				Entry("fault-injections", testCase{
+					typ:             "fault-injection",
+					name:            "web",
+					resource:        func() core_model.Resource { return mesh_core.NewFaultInjectionResource() },
+					expectedMessage: "deleted FaultInjection \"web\"\n",
+				}),
 				Entry("healthchecks", testCase{
 					typ:             "healthcheck",
 					name:            "web-to-backend",
 					resource:        func() core_model.Resource { return mesh_core.NewHealthCheckResource() },
 					expectedMessage: "deleted HealthCheck \"web-to-backend\"\n",
 				}),
+				Entry("proxytemplate", testCase{
+					typ:             "proxytemplate",
+					name:            "test-pt",
+					resource:        func() core_model.Resource { return mesh_core.NewProxyTemplateResource() },
+					expectedMessage: "deleted ProxyTemplate \"test-pt\"\n",
+				}),
 				Entry("retries", testCase{
 					typ:             "retry",
 					name:            "web-to-backend",
 					resource:        func() core_model.Resource { return mesh_core.NewRetryResource() },
 					expectedMessage: "deleted Retry \"web-to-backend\"\n",
 				}),
-				Entry("traffic-permissions", testCase{
-					typ:             "traffic-permission",
-					name:            "everyone-to-everyone",
-					resource:        func() core_model.Resource { return mesh_core.NewTrafficPermissionResource() },
-					expectedMessage: "deleted TrafficPermission \"everyone-to-everyone\"\n",
+				Entry("timeouts", testCase{
+					typ:             "timeout",
+					name:            "web",
+					resource:        func() core_model.Resource { return mesh_core.NewTimeoutResource() },
+					expectedMessage: "deleted Timeout \"web\"\n",
 				}),
 				Entry("traffic-logs", testCase{
 					typ:             "traffic-log",
@@ -174,6 +192,12 @@ var _ = Describe("kumactl delete ", func() {
 					resource:        func() core_model.Resource { return mesh_core.NewTrafficLogResource() },
 					expectedMessage: "deleted TrafficLog \"all-requests\"\n",
 				}),
+				Entry("traffic-permissions", testCase{
+					typ:             "traffic-permission",
+					name:            "everyone-to-everyone",
+					resource:        func() core_model.Resource { return mesh_core.NewTrafficPermissionResource() },
+					expectedMessage: "deleted TrafficPermission \"everyone-to-everyone\"\n",
+				}),
 				Entry("traffic-routes", testCase{
 					typ:             "traffic-route",
 					name:            "web-to-backend",
@@ -186,19 +210,7 @@ var _ = Describe("kumactl delete ", func() {
 					resource:        func() core_model.Resource { return mesh_core.NewTrafficTraceResource() },
 					expectedMessage: "deleted TrafficTrace \"web\"\n",
 				}),
-				Entry("fault-injections", testCase{
-					typ:             "fault-injection",
-					name:            "web",
-					resource:        func() core_model.Resource { return mesh_core.NewFaultInjectionResource() },
-					expectedMessage: "deleted FaultInjection \"web\"\n",
-				}),
-				Entry("circuit-breaker", testCase{
-					typ:             "circuit-breaker",
-					name:            "web",
-					resource:        func() core_model.Resource { return mesh_core.NewCircuitBreakerResource() },
-					expectedMessage: "deleted CircuitBreaker \"web\"\n",
-				}),
-				Entry("secret", testCase{
+				Entry("secrets", testCase{
 					typ:             "secret",
 					name:            "web",
 					resource:        func() core_model.Resource { return system.NewSecretResource() },
@@ -206,6 +218,58 @@ var _ = Describe("kumactl delete ", func() {
 				}),
 			)
 
+			DescribeTable("should succeed if resource exists",
+				func(given testCase) {
+					key := core_model.ResourceKey{Name: given.name}
+
+					By("creating resources necessary for the test")
+					// setup
+
+					// when
+					err := store.Create(context.Background(), given.resource(), core_store.CreateBy(key))
+					// then
+					Expect(err).ToNot(HaveOccurred())
+
+					By("running delete command")
+					// given
+					rootCmd.SetArgs([]string{
+						"--config-file", filepath.Join("..", "testdata", "sample-kumactl.config.yaml"),
+						"delete", given.typ, given.name})
+
+					// when
+					err = rootCmd.Execute()
+					// then
+					Expect(err).ToNot(HaveOccurred())
+					// and
+					Expect(errbuf.String()).To(BeEmpty())
+					Expect(outbuf.String()).To(Equal(given.expectedMessage))
+
+					By("verifying that resource under test was actually deleted")
+					// when
+					err = store.Get(context.Background(), given.resource(), core_store.GetBy(key))
+					// then
+					Expect(core_store.IsResourceNotFound(err)).To(BeTrue())
+				},
+				Entry("meshes", testCase{
+					typ:             "mesh",
+					name:            "test-mesh",
+					resource:        func() core_model.Resource { return mesh_core.NewMeshResource() },
+					expectedMessage: "deleted Mesh \"test-mesh\"\n",
+				}),
+				Entry("global-secrets", testCase{
+					typ:             "global-secret",
+					name:            "test-secret",
+					resource:        func() core_model.Resource { return system.NewGlobalSecretResource() },
+					expectedMessage: "deleted GlobalSecret \"test-secret\"\n",
+				}),
+				Entry("zones", testCase{
+					typ:             "zone",
+					name:            "eu-north",
+					resource:        func() core_model.Resource { return system.NewZoneResource() },
+					expectedMessage: "deleted Zone \"eu-north\"\n",
+				}),
+			)
+
 			DescribeTable("should fail if resource doesn't exist",
 				func(given testCase) {
 					By("running delete command")
diff --git a/app/kumactl/cmd/get/get_test.go b/app/kumactl/cmd/get/get_test.go
new file mode 100644
index 000000000..de35a5eff
--- /dev/null
+++ b/app/kumactl/cmd/get/get_test.go
@@ -0,0 +1,59 @@
+package get_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/spf13/cobra"
+
+	"github.com/kumahq/kuma/app/kumactl/cmd"
+	kumactl_cmd "github.com/kumahq/kuma/app/kumactl/pkg/cmd"
+	config_proto "github.com/kumahq/kuma/pkg/config/app/kumactl/v1alpha1"
+	core_store "github.com/kumahq/kuma/pkg/core/resources/store"
+	memory_resources "github.com/kumahq/kuma/pkg/plugins/resources/memory"
+)
+
+func hasSubCommand(cmd *cobra.Command, sub string) bool {
+	for _, c := range cmd.Commands() {
+		if c.Use == sub {
+			return true
+		}
+	}
+
+	return false
+}
+
+var _ = Describe("kumactl get ", func() {
+	Describe("Get Command", func() {
+		var rootCtx *kumactl_cmd.RootContext
+		var rootCmd, getCmd *cobra.Command
+		var store core_store.ResourceStore
+
+		BeforeEach(func() {
+			// setup
+			rootCtx = kumactl_cmd.DefaultRootContext()
+			rootCtx.Runtime.NewResourceStore = func(*config_proto.ControlPlaneCoordinates_ApiServer) (core_store.ResourceStore, error) {
+				return store, nil
+			}
+			store = core_store.NewPaginationStore(memory_resources.NewStore())
+
+			rootCmd = cmd.NewRootCmd(rootCtx)
+			for _, cmd := range rootCmd.Commands() {
+				if cmd.Use == "get" {
+					getCmd = cmd
+					break
+				}
+			}
+			Expect(getCmd).ToNot(BeNil())
+		})
+
+		It("should have get commands for all defined types", func() {
+			// when
+			Expect(len(getCmd.Commands()) > len(rootCtx.TypeArgs)).To(BeTrue())
+
+			// then
+			for sub := range rootCtx.TypeArgs {
+				Expect(hasSubCommand(getCmd, sub+" NAME")).To(BeTrue(), "failing to find "+sub)
+			}
+		})
+	})
+})
diff --git a/pkg/kds/server/server_test.go b/pkg/kds/server/server_test.go
index 9b5a2400a..37e27364e 100644
--- a/pkg/kds/server/server_test.go
+++ b/pkg/kds/server/server_test.go
@@ -61,43 +61,42 @@ var _ = Describe("KDS Server", func() {
 	It("should support all existing resource types", func() {
 		ctx := context.Background()
 
-		// Just to don't forget to update this test after updating 'kds.SupportedTypes
+		// Do not forget to update this test after updating 'kds.SupportedTypes
 		Expect([]proto.Message{
-			kds_samples.Mesh1,
-			kds_samples.Ingress,
+			kds_samples.CircuitBreaker,
+			kds_samples.Ingress, // mesh.DataplaneType
 			kds_samples.DataplaneInsight,
 			kds_samples.ExternalService,
-			kds_samples.CircuitBreaker,
 			kds_samples.FaultInjection,
 			kds_samples.HealthCheck,
+			kds_samples.Mesh1,
+			kds_samples.ProxyTemplate,
+			kds_samples.Retry,
+			kds_samples.Timeout,
 			kds_samples.TrafficLog,
 			kds_samples.TrafficPermission,
 			kds_samples.TrafficRoute,
 			kds_samples.TrafficTrace,
-			kds_samples.ProxyTemplate,
-			kds_samples.Retry,
 			kds_samples.Secret,
-			kds_samples.Config}).
+			kds_samples.Config,
+		}).
 			To(HaveLen(len(kds.SupportedTypes)))
 
 		vrf := kds_verifier.New().
-			Exec(kds_verifier.Create(ctx, &mesh.MeshResource{Spec: kds_samples.Mesh1}, store.CreateByKey("mesh-1", model.NoMesh))).
+			Exec(kds_verifier.Create(ctx, &mesh.CircuitBreakerResource{Spec: kds_samples.CircuitBreaker}, store.CreateByKey("cb-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.DataplaneResource{Spec: kds_samples.Ingress}, store.CreateByKey("Ingress-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.DataplaneInsightResource{Spec: kds_samples.DataplaneInsight}, store.CreateByKey("insight-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.ExternalServiceResource{Spec: kds_samples.ExternalService}, store.CreateByKey("es-1", "mesh-1"))).
-			Exec(kds_verifier.Create(ctx, &mesh.CircuitBreakerResource{Spec: kds_samples.CircuitBreaker}, store.CreateByKey("cb-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.FaultInjectionResource{Spec: kds_samples.FaultInjection}, store.CreateByKey("fi-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.HealthCheckResource{Spec: kds_samples.HealthCheck}, store.CreateByKey("hc-1", "mesh-1"))).
+			Exec(kds_verifier.Create(ctx, &mesh.MeshResource{Spec: kds_samples.Mesh1}, store.CreateByKey("mesh-1", model.NoMesh))).
+			Exec(kds_verifier.Create(ctx, &mesh.ProxyTemplateResource{Spec: kds_samples.ProxyTemplate}, store.CreateByKey("pt-1", "mesh-1"))).
+			Exec(kds_verifier.Create(ctx, &mesh.RetryResource{Spec: kds_samples.Retry}, store.CreateByKey("retry-1", "mesh-1"))).
+			Exec(kds_verifier.Create(ctx, &mesh.TimeoutResource{Spec: kds_samples.Timeout}, store.CreateByKey("timeout-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.TrafficLogResource{Spec: kds_samples.TrafficLog}, store.CreateByKey("tl-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.TrafficPermissionResource{Spec: kds_samples.TrafficPermission}, store.CreateByKey("tp-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.TrafficRouteResource{Spec: kds_samples.TrafficRoute}, store.CreateByKey("tr-1", "mesh-1"))).
 			Exec(kds_verifier.Create(ctx, &mesh.TrafficTraceResource{Spec: kds_samples.TrafficTrace}, store.CreateByKey("tt-1", "mesh-1"))).
-			Exec(kds_verifier.Create(ctx, &mesh.ProxyTemplateResource{Spec: kds_samples.ProxyTemplate}, store.CreateByKey("pt-1", "mesh-1"))).
-			Exec(kds_verifier.Create(
-				ctx,
-				&mesh.RetryResource{Spec: kds_samples.Retry},
-				store.CreateByKey("r-1", "mesh-1"),
-			)).
 			Exec(kds_verifier.Create(ctx, &system.SecretResource{Spec: kds_samples.Secret}, store.CreateByKey("s-1", "mesh-1"))).
 			Exec(kds_verifier.DiscoveryRequest(node, mesh.MeshType)).
 			Exec(kds_verifier.WaitResponse(defaultTimeout, func(rs []model.Resource) {
@@ -164,6 +163,16 @@ var _ = Describe("KDS Server", func() {
 				Expect(rs).To(HaveLen(1))
 				Expect(rs[0].GetSpec()).To(MatchProto(kds_samples.Secret))
 			})).
+			Exec(kds_verifier.DiscoveryRequest(node, mesh.RetryType)).
+			Exec(kds_verifier.WaitResponse(defaultTimeout, func(rs []model.Resource) {
+				Expect(rs).To(HaveLen(1))
+				Expect(rs[0].GetSpec()).To(MatchProto(kds_samples.Retry))
+			})).
+			Exec(kds_verifier.DiscoveryRequest(node, mesh.TimeoutType)).
+			Exec(kds_verifier.WaitResponse(defaultTimeout, func(rs []model.Resource) {
+				Expect(rs).To(HaveLen(1))
+				Expect(rs[0].GetSpec()).To(MatchProto(kds_samples.Timeout))
+			})).
 			Exec(kds_verifier.CloseStream())
 
 		err := vrf.Verify(tc)
diff --git a/pkg/test/kds/samples/resources.go b/pkg/test/kds/samples/resources.go
index 0c87932df..d5d9cf3e7 100644
--- a/pkg/test/kds/samples/resources.go
+++ b/pkg/test/kds/samples/resources.go
@@ -231,6 +231,45 @@ var (
 			},
 		},
 	}
+	Timeout = &mesh_proto.Timeout{
+		Sources: []*mesh_proto.Selector{{
+			Match: map[string]string{
+				"service": "*",
+			},
+		}},
+		Destinations: []*mesh_proto.Selector{{
+			Match: map[string]string{
+				"service": "*",
+			},
+		}},
+		Conf: &mesh_proto.Timeout_Conf{
+
+			ConnectTimeout: &duration.Duration{
+				Seconds: 5,
+			},
+			Tcp: &mesh_proto.Timeout_Conf_Tcp{
+				IdleTimeout: &duration.Duration{
+					Seconds: 5,
+				},
+			},
+			Http: &mesh_proto.Timeout_Conf_Http{
+				RequestTimeout: &duration.Duration{
+					Seconds: 5,
+				},
+				IdleTimeout: &duration.Duration{
+					Seconds: 5,
+				},
+			},
+			Grpc: &mesh_proto.Timeout_Conf_Grpc{
+				StreamIdleTimeout: &duration.Duration{
+					Seconds: 5,
+				},
+				MaxStreamDuration: &duration.Duration{
+					Seconds: 5,
+				},
+			},
+		},
+	}
 	Secret = &system_proto.Secret{
 		Data: &wrappers.BytesValue{Value: []byte("secret key")},
 	}