Merge branch 'asheshvidyut/NET-3865' of github.com:hashicorp/consul i…

…nto asheshvidyut/NET-3865
hashicorp · Jun 8, 2023 · 360ab9e · 360ab9e
2 parents 8e1f728 + eb69358
commit 360ab9e
Show file tree

Hide file tree

Showing 106 changed files with 6,821 additions and 198 deletions.
diff --git a/.changelog/17577.txt b/.changelog/17577.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+fix metric names in /docs/agent/telemetry
+```
diff --git a/.changelog/17581.txt b/.changelog/17581.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+gateways: **(Enterprise only)** Fixed a bug in API gateways where gateway configuration objects in non-default partitions did not reconcile properly.
+```
diff --git a/agent/consul/gateways/controller_gateways.go b/agent/consul/gateways/controller_gateways.go
@@ -93,7 +93,7 @@ func (r *apiGatewayReconciler) enqueueCertificateReferencedGateways(store *state
 	logger.Trace("certificate changed, enqueueing dependent gateways")
 	defer logger.Trace("finished enqueuing gateways")
 
-	_, entries, err := store.ConfigEntriesByKind(nil, structs.APIGateway, acl.WildcardEnterpriseMeta())
+	_, entries, err := store.ConfigEntriesByKind(nil, structs.APIGateway, wildcardMeta())
 	if err != nil {
 		logger.Warn("error retrieving api gateways", "error", err)
 		return err
@@ -564,11 +564,11 @@ type gatewayMeta struct {
 // tuples based on the state coming from the store. Any gateway that does not have
 // a corresponding bound-api-gateway config entry will be filtered out.
 func getAllGatewayMeta(store *state.Store) ([]*gatewayMeta, error) {
-	_, gateways, err := store.ConfigEntriesByKind(nil, structs.APIGateway, acl.WildcardEnterpriseMeta())
+	_, gateways, err := store.ConfigEntriesByKind(nil, structs.APIGateway, wildcardMeta())
 	if err != nil {
 		return nil, err
 	}
-	_, boundGateways, err := store.ConfigEntriesByKind(nil, structs.BoundAPIGateway, acl.WildcardEnterpriseMeta())
+	_, boundGateways, err := store.ConfigEntriesByKind(nil, structs.BoundAPIGateway, wildcardMeta())
 	if err != nil {
 		return nil, err
 	}
@@ -1074,12 +1074,12 @@ func requestToResourceRef(req controller.Request) structs.ResourceReference {
 
 // retrieveAllRoutesFromStore retrieves all HTTP and TCP routes from the given store
 func retrieveAllRoutesFromStore(store *state.Store) ([]structs.BoundRoute, error) {
-	_, httpRoutes, err := store.ConfigEntriesByKind(nil, structs.HTTPRoute, acl.WildcardEnterpriseMeta())
+	_, httpRoutes, err := store.ConfigEntriesByKind(nil, structs.HTTPRoute, wildcardMeta())
 	if err != nil {
 		return nil, err
 	}
 
-	_, tcpRoutes, err := store.ConfigEntriesByKind(nil, structs.TCPRoute, acl.WildcardEnterpriseMeta())
+	_, tcpRoutes, err := store.ConfigEntriesByKind(nil, structs.TCPRoute, wildcardMeta())
 	if err != nil {
 		return nil, err
 	}
@@ -1141,3 +1141,9 @@ func routeLogger(logger hclog.Logger, route structs.ConfigEntry) hclog.Logger {
 	meta := route.GetEnterpriseMeta()
 	return logger.With("route.kind", route.GetKind(), "route.name", route.GetName(), "route.namespace", meta.NamespaceOrDefault(), "route.partition", meta.PartitionOrDefault())
 }
+
+func wildcardMeta() *acl.EnterpriseMeta {
+	meta := acl.WildcardEnterpriseMeta()
+	meta.OverridePartition(acl.WildcardPartitionName)
+	return meta
+}
diff --git a/agent/envoyextensions/builtin/property-override/property_override.go b/agent/envoyextensions/builtin/property-override/property_override.go
@@ -7,6 +7,7 @@ import (
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
+	"github.com/hashicorp/consul/lib/decode"
 	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/mapstructure"
 	"google.golang.org/protobuf/proto"
@@ -73,7 +74,7 @@ func matchesResourceFilter[K proto.Message](rf ResourceFilter, resourceType Reso
 }
 
 type ServiceName struct {
-	api.CompoundServiceName
+	api.CompoundServiceName `mapstructure:",squash"`
 }
 
 // ResourceType is the type of Envoy resource being patched.
@@ -275,7 +276,21 @@ func Constructor(ext api.EnvoyExtension) (extensioncommon.EnvoyExtender, error)
 	if name := ext.Name; name != api.BuiltinPropertyOverrideExtension {
 		return nil, fmt.Errorf("expected extension name %q but got %q", api.BuiltinPropertyOverrideExtension, name)
 	}
-	if err := mapstructure.WeakDecode(ext.Arguments, &p); err != nil {
+	// This avoids issues with decoding nested slices, which are error-prone
+	// due to slice<->map coercion by mapstructure. See HookWeakDecodeFromSlice
+	// and WeaklyTypedInput docs for more details.
+	d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
+		DecodeHook: mapstructure.ComposeDecodeHookFunc(
+			decode.HookWeakDecodeFromSlice,
+			decode.HookTranslateKeys,
+		),
+		WeaklyTypedInput: true,
+		Result:           &p,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("error configuring decoder: %v", err)
+	}
+	if err := d.Decode(ext.Arguments); err != nil {
 		return nil, fmt.Errorf("error decoding extension arguments: %v", err)
 	}
 	if err := p.validate(); err != nil {

diff --git a/agent/envoyextensions/builtin/property-override/property_override_test.go b/agent/envoyextensions/builtin/property-override/property_override_test.go
@@ -220,8 +220,8 @@ func TestConstructor(t *testing.T) {
 			arguments: makeArguments(map[string]any{"Patches": []map[string]any{
 				makePatch(map[string]any{
 					"ResourceFilter": makeResourceFilter(map[string]any{
-						"Services": []ServiceName{
-							{CompoundServiceName: api.CompoundServiceName{}},
+						"Services": []map[string]any{
+							{},
 						},
 					}),
 				}),
@@ -240,6 +240,42 @@ func TestConstructor(t *testing.T) {
 			expected:  validTestCase(OpAdd, extensioncommon.TrafficDirectionOutbound, ResourceTypeRoute).expected,
 			ok:        true,
 		},
+		// Ensure that embedded api struct used for Services is parsed correctly.
+		// See also above comment on decode.HookWeakDecodeFromSlice.
+		"single value Services decoded as map construction succeeds": {
+			arguments: makeArguments(map[string]any{"Patches": []map[string]any{
+				makePatch(map[string]any{
+					"ResourceFilter": makeResourceFilter(map[string]any{
+						"Services": []map[string]any{
+							{"Name": "foo"},
+						},
+					}),
+				}),
+			}}),
+			expected: propertyOverride{
+				Patches: []Patch{
+					{
+						ResourceFilter: ResourceFilter{
+							ResourceType:     ResourceTypeRoute,
+							TrafficDirection: extensioncommon.TrafficDirectionOutbound,
+							Services: []*ServiceName{
+								{CompoundServiceName: api.CompoundServiceName{
+									Name:      "foo",
+									Namespace: "default",
+									Partition: "default",
+								}},
+							},
+						},
+						Op:    OpAdd,
+						Path:  "/name",
+						Value: "foo",
+					},
+				},
+				Debug:     true,
+				ProxyType: api.ServiceKindConnectProxy,
+			},
+			ok: true,
+		},
 		"invalid ProxyType": {
 			arguments: makeArguments(map[string]any{
 				"Patches": []map[string]any{

diff --git a/go.mod b/go.mod
@@ -103,6 +103,7 @@ require (
 	go.opentelemetry.io/proto/otlp v0.19.0
 	go.uber.org/goleak v1.1.10
 	golang.org/x/crypto v0.1.0
+	golang.org/x/exp v0.0.0-20230321023759-10a507213a29
 	golang.org/x/net v0.10.0
 	golang.org/x/oauth2 v0.6.0
 	golang.org/x/sync v0.2.0
@@ -240,7 +241,6 @@ require (
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/otel/trace v1.16.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
-	golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
 	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
 	golang.org/x/mod v0.10.0 // indirect
 	golang.org/x/term v0.8.0 // indirect

diff --git a/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-service.json b/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-service.json
@@ -0,0 +1,37 @@
+{
+   "id": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "Service"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api"
+   },
+   "data": {
+      "@type": "hashicorp.consul.catalog.v1alpha1.Service",
+      "workloads": {
+         "prefixes": [
+            "api-"
+         ]
+      },
+      "ports": [
+         {
+            "target_port": "http",
+            "protocol": "PROTOCOL_HTTP"
+         },
+         {
+            "target_port": "grpc",
+            "protocol": "PROTOCOL_GRPC"
+         },
+         {
+            "target_port": "mesh",
+            "protocol": "PROTOCOL_MESH"
+         }
+      ]
+   }
+}
diff --git a/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-1-health.json b/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-1-health.json
@@ -0,0 +1,33 @@
+{
+   "id": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "HealthStatus"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-1-health"
+   },
+   "owner": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "Workload"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-1"
+   },
+   "data": {
+      "@type": "hashicorp.consul.catalog.v1alpha1.HealthStatus",
+      "type": "synthetic",
+      "status": "HEALTH_PASSING"
+   }
+}
diff --git a/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-1.json b/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-1.json
@@ -0,0 +1,46 @@
+{
+   "id": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "Workload"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-1"
+   },
+   "data": {
+      "@type": "hashicorp.consul.catalog.v1alpha1.Workload",
+      "addresses": [
+         {
+            "host": "172.16.1.1"
+         },
+         {
+            "host": "198.18.2.1",
+            "external": true,
+            "ports": [
+               "mesh"
+            ]
+         }
+      ],
+      "ports": {
+         "http": {
+            "port": 8080,
+            "protocol": "PROTOCOL_HTTP"
+         },
+         "grpc": {
+            "port": 9090,
+            "protocol": "PROTOCOL_GRPC"
+         },
+         "mesh": {
+            "port": 10000,
+            "protocol": "PROTOCOL_MESH"
+         }
+      },
+      "identity": "api",
+      "node_name": "node-1"
+   }
+}
diff --git a/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-10-health.json b/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-10-health.json
@@ -0,0 +1,33 @@
+{
+   "id": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "HealthStatus"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-10-health"
+   },
+   "owner": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "Workload"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-10"
+   },
+   "data": {
+      "@type": "hashicorp.consul.catalog.v1alpha1.HealthStatus",
+      "type": "synthetic",
+      "status": "HEALTH_WARNING"
+   }
+}
diff --git a/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-10.json b/internal/catalog/catalogtest/integration_test_data/v1alpha1/api-workload-10.json
@@ -0,0 +1,46 @@
+{
+   "id": {
+      "type": {
+         "group": "catalog",
+         "group_version": "v1alpha1",
+         "kind": "Workload"
+      },
+      "tenancy": {
+         "partition": "default",
+         "namespace": "default",
+         "peer_name": "local"
+      },
+      "name": "api-10"
+   },
+   "data": {
+      "@type": "hashicorp.consul.catalog.v1alpha1.Workload",
+      "addresses": [
+         {
+            "host": "172.16.1.10"
+         },
+         {
+            "host": "198.18.2.10",
+            "external": true,
+            "ports": [
+               "mesh"
+            ]
+         }
+      ],
+      "ports": {
+         "http": {
+            "port": 8080,
+            "protocol": "PROTOCOL_HTTP"
+         },
+         "grpc": {
+            "port": 9090,
+            "protocol": "PROTOCOL_GRPC"
+         },
+         "mesh": {
+            "port": 10000,
+            "protocol": "PROTOCOL_MESH"
+         }
+      },
+      "identity": "api",
+      "node_name": "node-3"
+   }
+}