diff --git a/CHANGELOG.md b/CHANGELOG.md index ae3e266c2d..69ca6ac350 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,7 +91,7 @@ it will be removed; but as it won't be user-visible this isn't considered a brea ### Emissary-ingress and Ambassador Edge Stack - Feature: This upgrades Emissary-ingress to be built on Envoy v1.28.0 which provides security, - performance and feature enhancements. You can read more about them here: Envoy Proxy 1.28.0 Release Notes @@ -103,37 +103,42 @@ it will be removed; but as it won't be user-visible this isn't considered a brea - Change: Upgraded Emissary-ingress to the latest release of Golang as part of our general dependency upgrade process. +- Bugfix: Emissary-ingress was incorrectly caching Mappings with regex headers using the header name + instead of the Mapping name, which could reduce the cache's effectiveness. This has been fixed so + that the correct key is used. ([Incorrect Cache Key for Mapping]) + +[Incorrect Cache Key for Mapping]: https://github.com/emissary-ingress/emissary/issues/5714 + ## [3.9.0] November 13, 2023 [3.9.0]: https://github.com/emissary-ingress/emissary/compare/v3.8.0...v3.9.0 ### Emissary-ingress and Ambassador Edge Stack - Feature: This upgrades Emissary-ingress to be built on Envoy v1.27.2 which provides security, - performance and feature enhancements. You can read more about them here: Envoy Proxy 1.27.2 Release Notes -- Feature: By default, Emissary-ingress will return an `UNAVAILABLE` code when a request using gRPC +- Feature: By default, Emissary-ingress will return an `UNAVAILABLE` code when a request using gRPC is rate limited. The `RateLimitService` resource now exposes a new - `grpc.use_resource_exhausted_code` field that when set to `true`, Emissary-ingress will return a - `RESOURCE_EXHAUSTED` gRPC code instead. Thanks to Jerome + `grpc.use_resource_exhausted_code` field that when set to `true`, Emissary-ingress will return a + `RESOURCE_EXHAUSTED` gRPC code instead. Thanks to Jerome Froelich for contributing this feature! - Feature: Envoy runtime fields that were provided to mitigate the recent HTTP/2 rapid reset - vulnerability can now be configured via the Module resource so the configuration will persist - between restarts. This configuration is added to the Envoy bootstrap config, so restarting - Emissary is necessary after changing these fields for the configuration to take effect. + vulnerability can now be configured via the Module resource so the configuration will persist + between restarts. This configuration is added to the Envoy bootstrap config, so restarting + Emissary is necessary after changing these fields for the configuration to take effect. - Change: APIExt would previously allow for TLS 1.0 connections. We have updated it to now only use - a minimum TLS version of 1.3 to resolve security concerns. + a minimum TLS version of 1.3 to resolve security concerns. - Change: - Update default image to Emissary-ingress v3.9.0.
- Bugfix: The APIExt server provides CRD conversion between the stored version v2 and the version - watched for by Emissary-ingress v3alpha1. Since this component is required to operate - Emissary-ingress, we have introduced an init container that will ensure it is available before - starting. This will help address some of the intermittent issues seen during install and - upgrades. + watched for by Emissary-ingress v3alpha1. Since this component is required to operate + Emissary-ingress, we have introduced an init container that will ensure it is available before + starting. This will help address some of the intermittent issues seen during install and upgrades. ## [3.8.0] August 29, 2023 [3.8.0]: https://github.com/emissary-ingress/emissary/compare/v3.7.2...v3.8.0 diff --git a/cmd/entrypoint/irtype.go b/cmd/entrypoint/irtype.go new file mode 100644 index 0000000000..90e57cfcb8 --- /dev/null +++ b/cmd/entrypoint/irtype.go @@ -0,0 +1,177 @@ +package entrypoint + +import ( + "encoding/json" + "fmt" +) + +type IRResource struct { + Active bool `json:"_active"` + CacheKey string `json:"_cache_key,omitempty"` + Errored bool `json:"_errored"` + ReferencedBy []string `json:"_referenced_by,omitempty"` + RKey string `json:"_rkey,omitempty"` + Location string `json:"location,omitempty"` + Kind string `json:"kind"` + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` +} + +type IRClusterHealthCheck struct { + IRResource +} + +type IRClusterTarget struct { + IP string `json:"ip"` + Port int `json:"port"` + TargetKind string `json:"target_kind"` +} + +type IRCluster struct { + IRResource + BarHostname string `json:"_hostname"` // Why this _and_ hostname? + BarNamespace string `json:"_namespace"` // Why this _and_ namespace? + Port int `json:"_port"` + Resolver string `json:"_resolver"` + ConnectTimeoutMs int `json:"connect_timeout_ms"` + EnableEndpoints bool `json:"enable_endpoints"` + EnableIPv4 bool `json:"enable_ipv4"` + EnableIPv6 bool `json:"enable_ipv6"` + EnvoyName string `json:"envoy_name"` + HealthChecks IRClusterHealthCheck `json:"health_checks,omitempty"` + IgnoreCluster bool `json:"ignore_cluster"` + LBType string `json:"lb_type"` + RespectDNSTTL bool `json:"respect_dns_ttl"` + Service string `json:"service"` + StatsName string `json:"stats_name"` + Targets []IRClusterTarget `json:"targets"` + Type string `json:"type"` + URLs []string `json:"urls"` +} + +type IRQueryParameter struct { + Name string `json:"name,omitempty"` + Value string `json:"value,omitempty"` + Regex bool `json:"regex,omitempty"` +} + +type IRRegexRewrite struct { + Pattern string `json:"pattern,omitempty"` + Substitution string `json:"substitution,omitempty"` +} + +// Route weights are really annoying: in Python they're a +// List[Union[str, int]], which is a pain to represent in Go. + +type IRRouteWeightElement struct { + Int int + Str string +} + +type IRRouteWeight []IRRouteWeightElement + +func (rw IRRouteWeight) MarshalJSON() ([]byte, error) { + arr := make([]interface{}, len(rw)) + + for i, elem := range rw { + if elem.Str != "" { + arr[i] = elem.Str + } else { + arr[i] = elem.Int + } + } + + return json.Marshal(arr) +} + +func (rw *IRRouteWeight) UnmarshalJSON(data []byte) error { + var arr []interface{} + + if err := json.Unmarshal(data, &arr); err != nil { + return err + } + + *rw = make([]IRRouteWeightElement, len(arr)) + + for i, elem := range arr { + switch v := elem.(type) { + case string: + (*rw)[i] = IRRouteWeightElement{Str: v} + case float64: + (*rw)[i] = IRRouteWeightElement{Int: int(v)} + default: + return fmt.Errorf("unexpected type in IRRouteWeight: %T", elem) + } + } + + return nil +} + +type IRMapping struct { + IRResource + // CumulativeWeight is the _computed_ cumulative weight assigned to a + // Mapping within its groups; Weight is the value specified in the + // Mapping's spec.weight (it's a pointer because that value is optional). + // + // If you have mappings in a Group with weights 10, 40, and unset, the + // Weights will be pointer-to-10, pointer-to-40, and nil, and the + // CumulativeWeights will be 10, 50, and 100. + CumulativeWeight int `json:"_weight"` + Weight *int `json:"weight"` + Cluster IRCluster `json:"cluster"` + ClusterKey string `json:"cluster_key"` + DefaultClass string `json:"default_class"` + GroupID string `json:"group_id"` + Headers []IRHeader `json:"headers"` + Host string `json:"host"` + Precedence int `json:"precedence"` + Prefix string `json:"prefix"` + QueryParameters []IRQueryParameter `json:"query_parameters,omitempty"` + RegexRewrite IRRegexRewrite `json:"regex_rewrite,omitempty"` + Resolver string `json:"resolver"` + Rewrite string `json:"rewrite"` + RouteWeight IRRouteWeight `json:"route_weight"` + Service string `json:"service"` + TimeoutMS int `json:"timeout_ms"` +} + +type IRRequestPolicy struct { + Action string `json:"action"` +} + +type IRHost struct { + IRResource + Hostname string `json:"hostname"` + InsecureAction string `json:"insecure_action"` + RequestPolicy map[string]IRRequestPolicy `json:"requestPolicy"` // Yes, really. + SecureAction string `json:"secure_action"` + SNI string `json:"sni"` +} + +type IRHeader struct { + Name string `json:"name"` + Regex bool `json:"regex"` + Value string `json:"value"` +} + +type IRGroup struct { + IRResource + DefaultClass string `json:"default_class"` + GroupID string `json:"group_id"` + GroupWeight IRRouteWeight `json:"group_weight"` + Headers []IRHeader `json:"headers"` + Host string `json:"host"` + Mappings []IRMapping `json:"mappings"` + Precedence int `json:"precedence"` + Prefix string `json:"prefix"` + QueryParameters []IRQueryParameter `json:"query_parameters"` + RegexRewrite IRRegexRewrite `json:"regex_rewrite"` + Rewrite string `json:"rewrite"` + TimeoutMS int `json:"timeout_ms"` +} + +type IR struct { + Clusters map[string]IRCluster `json:"clusters"` + Groups []IRGroup `json:"groups"` + Hosts []IRHost `json:"hosts"` +} diff --git a/cmd/entrypoint/irtype_test.go b/cmd/entrypoint/irtype_test.go new file mode 100644 index 0000000000..6488a7e9b5 --- /dev/null +++ b/cmd/entrypoint/irtype_test.go @@ -0,0 +1,153 @@ +package entrypoint_test + +import ( + "encoding/json" + "testing" + + "github.com/emissary-ingress/emissary/v3/cmd/entrypoint" + "github.com/stretchr/testify/assert" +) + +func TestIRRouteWeight(t *testing.T) { + rw := entrypoint.IRRouteWeight{ + {Int: 1}, + {Str: "foo"}, + {Int: 2}, + } + + // MarshalJSON + expectedJSON := `[1,"foo",2]` + actualJSON, err := json.Marshal(rw) + assert.Nil(t, err) + assert.Equal(t, expectedJSON, string(actualJSON)) + + var check entrypoint.IRRouteWeight + err = json.Unmarshal(actualJSON, &check) + assert.Nil(t, err) + assert.Equal(t, rw, check) + + // UnmarshalJSON + jsonData := []byte(`[1,"bar",3]`) + expectedRW := entrypoint.IRRouteWeight{ + {Int: 1}, + {Str: "bar"}, + {Int: 3}, + } + var actualRW entrypoint.IRRouteWeight + err = json.Unmarshal(jsonData, &actualRW) + assert.Nil(t, err) + assert.Equal(t, expectedRW, actualRW) +} + +func TestIRCluster(t *testing.T) { + clusterJSON := `{ + "_active": true, + "_cache_key": "Cluster-cluster_127_0_0_1_8877_default", + "_errored": false, + "_hostname": "127.0.0.1", + "_namespace": "default", + "_port": 8877, + "_referenced_by": [ + "--internal--" + ], + "_resolver": "kubernetes-service", + "_rkey": "cluster_127_0_0_1_8877_default", + "connect_timeout_ms": 3000, + "enable_endpoints": false, + "enable_ipv4": true, + "enable_ipv6": false, + "envoy_name": "cluster_127_0_0_1_8877_default", + "health_checks": { + "_active": true, + "_errored": false, + "_rkey": "ir.health_checks", + "kind": "IRHealthChecks", + "location": "--internal--", + "name": "health_checks", + "namespace": "default" + }, + "ignore_cluster": false, + "kind": "IRCluster", + "lb_type": "round_robin", + "location": "--internal--", + "name": "cluster_127_0_0_1_8877_default", + "namespace": "default", + "respect_dns_ttl": false, + "service": "127.0.0.1:8877", + "stats_name": "127_0_0_1_8877", + "targets": [ + { + "ip": "127.0.0.1", + "port": 8877, + "target_kind": "IPaddr" + } + ], + "type": "strict_dns", + "urls": [ + "tcp://127.0.0.1:8877" + ] + }` + + expectedCluster := entrypoint.IRCluster{ + IRResource: entrypoint.IRResource{ + Active: true, + CacheKey: "Cluster-cluster_127_0_0_1_8877_default", + Errored: false, + ReferencedBy: []string{"--internal--"}, + RKey: "cluster_127_0_0_1_8877_default", + Location: "--internal--", + Kind: "IRCluster", + Name: "cluster_127_0_0_1_8877_default", + Namespace: "default", + }, + BarHostname: "127.0.0.1", + BarNamespace: "default", + Port: 8877, + Resolver: "kubernetes-service", + ConnectTimeoutMs: 3000, + EnableEndpoints: false, + EnableIPv4: true, + EnableIPv6: false, + EnvoyName: "cluster_127_0_0_1_8877_default", + HealthChecks: entrypoint.IRClusterHealthCheck{ + IRResource: entrypoint.IRResource{ + Active: true, + Errored: false, + RKey: "ir.health_checks", + Kind: "IRHealthChecks", + Location: "--internal--", + Name: "health_checks", + Namespace: "default", + }, + }, + IgnoreCluster: false, + LBType: "round_robin", + RespectDNSTTL: false, + Service: "127.0.0.1:8877", + StatsName: "127_0_0_1_8877", + Targets: []entrypoint.IRClusterTarget{ + { + IP: "127.0.0.1", + Port: 8877, + TargetKind: "IPaddr", + }, + }, + Type: "strict_dns", + URLs: []string{ + "tcp://127.0.0.1:8877", + }, + } + + var unmarshaledCluster entrypoint.IRCluster + err := json.Unmarshal([]byte(clusterJSON), &unmarshaledCluster) + assert.Nil(t, err) + assert.Equal(t, expectedCluster, unmarshaledCluster) + + remarshaledJSON, err := json.Marshal(unmarshaledCluster) + assert.Nil(t, err) + + var unmarshaledCluster2 entrypoint.IRCluster + err = json.Unmarshal(remarshaledJSON, &unmarshaledCluster2) + assert.Nil(t, err) + assert.Equal(t, expectedCluster, unmarshaledCluster2) +} diff --git a/cmd/entrypoint/testdata/unrelated-mappings/host.yaml b/cmd/entrypoint/testdata/unrelated-mappings/host.yaml new file mode 100644 index 0000000000..2356c0c662 --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/host.yaml @@ -0,0 +1,12 @@ +apiVersion: getambassador.io/v2 +kind: Host +metadata: + name: origin-test-app-emissariy.spike-pp.test-labs.com + namespace: infrastructure +spec: + ambassador_id: + - --apiVersion-v3alpha1-only--default + hostname: origin-test-app-emissariy.spike-pp.test-labs.com + requestPolicy: + insecure: + action: Route diff --git a/cmd/entrypoint/testdata/unrelated-mappings/mapping1.yaml b/cmd/entrypoint/testdata/unrelated-mappings/mapping1.yaml new file mode 100644 index 0000000000..842e97a9d9 --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/mapping1.yaml @@ -0,0 +1,33 @@ + +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: workload1 + name: workload1-mapping + namespace: infrastructure +spec: + bypass_auth: true + host: test.example.com + prefix: / + rewrite: "" + service: workload1 + timeout_ms: 3000 + +--- + +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: workload2 + name: workload2-mapping + namespace: infrastructure +spec: + bypass_auth: true + host: test.example.com + prefix: / + rewrite: "" + service: workload2 + timeout_ms: 3000 + weight: 10 diff --git a/cmd/entrypoint/testdata/unrelated-mappings/mapping2.yaml b/cmd/entrypoint/testdata/unrelated-mappings/mapping2.yaml new file mode 100644 index 0000000000..031fc985aa --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/mapping2.yaml @@ -0,0 +1,16 @@ +--- +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: workload2 + name: workload2-mapping + namespace: infrastructure +spec: + bypass_auth: true + host: test.example.com + prefix: / + rewrite: "" + service: workload2 + timeout_ms: 3000 + weight: 50 diff --git a/cmd/entrypoint/testdata/unrelated-mappings/mapping3.yaml b/cmd/entrypoint/testdata/unrelated-mappings/mapping3.yaml new file mode 100644 index 0000000000..ec193a83b0 --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/mapping3.yaml @@ -0,0 +1,31 @@ + +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: workload1 + name: workload1-mapping + namespace: infrastructure +spec: + bypass_auth: true + host: test.example.com + prefix: / + rewrite: "" + service: workload1 + timeout_ms: 3000 + weight: 20 +--- +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: workload3 + name: workload3-mapping + namespace: infrastructure +spec: + bypass_auth: true + host: test.example.com + prefix: / + rewrite: "" + service: workload3 + timeout_ms: 3000 diff --git a/cmd/entrypoint/testdata/unrelated-mappings/service.yaml b/cmd/entrypoint/testdata/unrelated-mappings/service.yaml new file mode 100644 index 0000000000..bcd10c0ddb --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/service.yaml @@ -0,0 +1,37 @@ +apiVersion: v1 +kind: Service +metadata: + labels: + app: test-app-emissariy + name: test-app-emissariy-default + name: test-app-emissariy-default + namespace: infrastructure +spec: + ports: + - name: "80" + port: 80 + protocol: TCP + targetPort: 80 + selector: + app: test-app-emissariy + type: ClusterIP + +--- + +apiVersion: v1 +kind: Service +metadata: + labels: + app: canary-test-app-emissariy + name: canary-test-app-emissariy-default + name: canary-test-app-emissariy-default + namespace: infrastructure +spec: + ports: + - name: "80" + port: 80 + protocol: TCP + targetPort: 80 + selector: + app: canary-test-app-emissariy + type: ClusterIP \ No newline at end of file diff --git a/cmd/entrypoint/testdata/unrelated-mappings/unrelated.yaml b/cmd/entrypoint/testdata/unrelated-mappings/unrelated.yaml new file mode 100644 index 0000000000..b9b022668f --- /dev/null +++ b/cmd/entrypoint/testdata/unrelated-mappings/unrelated.yaml @@ -0,0 +1,16 @@ +--- +apiVersion: getambassador.io/v2 +kind: Mapping +metadata: + labels: + app: unrelated + name: unrelated + namespace: infrastructure +spec: + bypass_auth: true + host: unrelated.spike-pp.test-labs.com + prefix: / + resolver: endpoint + rewrite: "" + service: unrelated + timeout_ms: 3000 diff --git a/cmd/entrypoint/testutil_fake_test.go b/cmd/entrypoint/testutil_fake_test.go index 6569cb67e7..8f50870cea 100644 --- a/cmd/entrypoint/testutil_fake_test.go +++ b/cmd/entrypoint/testutil_fake_test.go @@ -4,9 +4,10 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" + "io" "net/http" "os" + "path/filepath" "reflect" "sync" "sync/atomic" @@ -74,6 +75,7 @@ type Fake struct { fastpath *testqueue.Queue // All fastpath snapshots that have been produced. snapshots *testqueue.Queue // All snapshots that have been produced. + irs *testqueue.Queue // All IRs that have been produced envoyConfigs *testqueue.Queue // All envoyConfigs that have been produced. // This is used to make Teardown idempotent. @@ -89,12 +91,17 @@ type FakeConfig struct { EnvoyConfig bool // If true then the Fake will produce envoy configs in addition to Snapshots. DiagdDebug bool // If true then diagd will have debugging enabled Timeout time.Duration // How long to wait for snapshots and/or envoy configs to become available. + OutputDir string // Directory for Emissary's output files } func (fc *FakeConfig) fillDefaults() { if fc.Timeout == 0 { fc.Timeout = 10 * time.Second } + + if fc.OutputDir == "" { + fc.OutputDir = "/tmp/mock" + } } // NewFake will construct a new Fake object. See RunFake for a convenient way to handle construct, @@ -120,6 +127,7 @@ func NewFake(t *testing.T, config FakeConfig) *Fake { fastpath: testqueue.NewQueue(t, config.Timeout), snapshots: testqueue.NewQueue(t, config.Timeout), + irs: testqueue.NewQueue(t, config.Timeout), envoyConfigs: testqueue.NewQueue(t, config.Timeout), } @@ -157,12 +165,18 @@ func (f *Fake) Setup() { f.DiagdBindPort = GetDiagdBindPort() + err = os.MkdirAll(f.config.OutputDir, os.ModePerm) + + if err != nil { + f.T.Fatalf("failed to create directory: %v", err) + } + f.group.Go("diagd", func(ctx context.Context) error { args := []string{ "diagd", - "/tmp", - "/tmp/bootstrap-ads.json", - "/tmp/envoy.json", + f.config.OutputDir, + filepath.Join(f.config.OutputDir, "bootstrap-ads.json"), + filepath.Join(f.config.OutputDir, "envoy.json"), "--no-envoy", "--host", "127.0.0.1", "--port", f.DiagdBindPort, @@ -230,7 +244,7 @@ func (f *Fake) GetFeatures(ctx context.Context, features interface{}) error { return fmt.Errorf("unexpected status code: %d", resp.StatusCode) } - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { return err @@ -299,9 +313,18 @@ func (f *Fake) AssertEndpointsEmpty(timeout time.Duration) { f.fastpath.AssertEmpty(f.T, timeout, "endpoints queue not empty") } +// SnapshotEntry contains a snapshot (the result of discovery), its +// corresponding IR and Envoy configs (if available, see below), and the +// disposition of the snapshot. +// +// The IR and Envoy configs will be available _only_ if the Fake harness was +// configured with EnvoyConfig set to true, and they are likely to be +// meaningful only if the disposition is SnapshotReady. type SnapshotEntry struct { - Disposition SnapshotDisposition - Snapshot *snapshot.Snapshot + Disposition SnapshotDisposition + Snapshot *snapshot.Snapshot + IRText []byte + EnvoyConfigText []byte } func (entry SnapshotEntry) String() string { @@ -314,20 +337,28 @@ func (entry SnapshotEntry) String() string { // We pass this into the watcher loop to get notified when a snapshot is produced. func (f *Fake) notifySnapshot(ctx context.Context, disp SnapshotDisposition, snapJSON []byte) error { + var envoyConfigText []byte + var irText []byte + if disp == SnapshotReady && f.config.EnvoyConfig { if err := notifyReconfigWebhooks(ctx, &noopNotable{}); err != nil { return err } - f.appendEnvoyConfig(ctx) + + irText = f.appendIR() + envoyConfigText = f.appendEnvoyConfig(ctx) } var snap *snapshot.Snapshot err := json.Unmarshal(snapJSON, &snap) if err != nil { - f.T.Fatalf("error decoding snapshot: %+v", err) + f.T.Errorf("error decoding snapshot: %+v", err) + return err } - f.snapshots.Add(f.T, SnapshotEntry{disp, snap}) + entry := SnapshotEntry{disp, snap, irText, envoyConfigText} + f.snapshots.Add(f.T, entry) + return nil } @@ -356,13 +387,54 @@ func (f *Fake) GetSnapshot(predicate func(*snapshot.Snapshot) bool) (*snapshot.S return entry.Snapshot, nil } -func (f *Fake) appendEnvoyConfig(ctx context.Context) { - msg, err := ambex.Decode(ctx, "/tmp/envoy.json") +func (f *Fake) appendIR() []byte { + irJSON, err := os.ReadFile(filepath.Join(f.config.OutputDir, "ir.json")) + if err != nil { + f.T.Fatalf("error reading ir.json: %+v", err) + } + + var ir IR + err = json.Unmarshal(irJSON, &ir) + if err != nil { + f.T.Fatalf("error unmarshaling ir.json: %+v", err) + } + + f.irs.Add(f.T, &ir) + + // Save the text of the IR, too, in with the snapshot. + return irJSON +} + +// GetIR will return the next IR that satisfies the supplied predicate. +func (f *Fake) GetIR(predicate func(*IR) bool) (*IR, error) { + f.T.Helper() + untyped, err := f.irs.Get(f.T, func(obj interface{}) bool { + ir := obj.(*IR) + return predicate(ir) + }) + if err != nil { + return nil, err + } + return untyped.(*IR), nil +} + +func (f *Fake) appendEnvoyConfig(ctx context.Context) []byte { + msg, err := ambex.Decode(ctx, filepath.Join(f.config.OutputDir, "envoy.json")) if err != nil { f.T.Fatalf("error decoding envoy.json after sending snapshot to python: %+v", err) } + bs := msg.(*v3bootstrap.Bootstrap) f.envoyConfigs.Add(f.T, bs) + + // Save the text of the Envoy config, too, in with the snapshot. + envoyConfig, err := os.ReadFile(filepath.Join(f.config.OutputDir, "envoy.json")) + + if err != nil { + f.T.Fatalf("error reading envoy.json: %+v", err) + } + + return envoyConfig } // GetEnvoyConfig will return the next envoy config that satisfies the supplied predicate. diff --git a/cmd/entrypoint/unrelated_mappings_test.go b/cmd/entrypoint/unrelated_mappings_test.go new file mode 100644 index 0000000000..db7bdf625a --- /dev/null +++ b/cmd/entrypoint/unrelated_mappings_test.go @@ -0,0 +1,203 @@ +package entrypoint_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/emissary-ingress/emissary/v3/cmd/entrypoint" +) + +func getWorkload1MappingGroup(ir *entrypoint.IR) (*entrypoint.IRGroup, bool) { + for _, group := range ir.Groups { + if group.Name == "GROUP: workload1-mapping" { + return &group, true + } + } + + return nil, false +} + +func predicate(ir *entrypoint.IR) bool { + _, ok := getWorkload1MappingGroup(ir) + + return ok +} + +type WeightCheck struct { + weight *int + cumulative int +} + +func NewWeightCheck(weight int, cumulative int) WeightCheck { + weightPtr := &weight + + if weight < 0 { + weightPtr = nil + } + + return WeightCheck{weight: weightPtr, cumulative: cumulative} +} + +// checkIR is a helper function that flushes the world, gets an IR, then +// checks the IR for the expected state. It'll find the "workload1-mapping" +// group and check that it has mappings for each entry in the weights map, +// with the correct weights. +func checkIR(f *entrypoint.Fake, what string, weights map[string]WeightCheck) { + // Flush the Fake harness so that we get a configuration. + f.Flush() + + // We need the IR from that configuration. + ir, err := f.GetIR(predicate) + require.NoError(f.T, err) + + // In the IR, we should find a group called "workload1-mapping". + group, ok := getWorkload1MappingGroup(ir) + require.True(f.T, ok) + + // That group should have the same number of mappings as we have entries + // in the weights map. + require.Len(f.T, group.Mappings, len(weights)) + + // Now we can check each mapping. Since we need all of them to be present + // in the group, we'll start with a set of all the mappings defined in the + // weights map, and remove them as we find them in the mapping. Any left + // over at the end were missing from the group. + missingMappings := make(map[string]struct{}) + for name := range weights { + missingMappings[name] = struct{}{} + } + + // Next, walk over the group's mappings and check against the weights map. + for _, mapping := range group.Mappings { + check, ok := weights[mapping.Name] + + if ok { + // It's present; remove it from the leftovers. + delete(missingMappings, mapping.Name) + + // Next, make sure the weights match. + var msg string + + if check.weight == nil { + if mapping.Weight != nil { + msg = fmt.Sprintf("%s: weight for %s should not be present but is %d", what, mapping.Name, *mapping.Weight) + } + } else if mapping.Weight == nil { + msg = fmt.Sprintf("%s: weight for %s should be %d but is not present", what, mapping.Name, *check.weight) + } else if *check.weight != *mapping.Weight { + msg = fmt.Sprintf("%s: unexpected weight for mapping %s: wanted %d, got %d", what, mapping.Name, *check.weight, mapping.Weight) + } + + if msg != "" { + for _, m := range group.Mappings { + msg += "\n" + + if m.Weight == nil { + msg += fmt.Sprintf(" - %s: weight unset", m.Name) + } else { + msg += fmt.Sprintf(" - %s: weight %d", m.Name, *m.Weight) + } + } + + f.T.Fatal(msg) + } + + // Finally, check the cumulative weight. + if check.cumulative != mapping.CumulativeWeight { + f.T.Fatalf("%s: unexpected cumulative weight for mapping %s: wanted %d, got %d", what, mapping.Name, check.cumulative, mapping.CumulativeWeight) + } + } else { + // It's not present; this is a problem. + f.T.Fatalf("%s: unexpected mapping: %#v", what, mapping.Name) + } + } + + // Finally, we should have no leftovers. + if len(missingMappings) > 0 { + msg := fmt.Sprintf("%s: missing mappings:", what) + + for name := range missingMappings { + msg += "\n" + msg += fmt.Sprintf(" - %s", name) + } + + f.T.Fatal(msg) + } +} + +// The Fake struct is a test harness for Emissary. See testutil_fake_test.go +// for details. Note that this test depends on diagd being in your path. If +// diagd is not available, the test will be skipped. + +func TestUnrelatedMappings(t *testing.T) { + // Use RunFake() to spin up the fake control plane, and note that we + // _must_ set EnvoyConfig true to do anything with IR. We need the IR + // for this test, so... + f := entrypoint.RunFake(t, entrypoint.FakeConfig{EnvoyConfig: true}, nil) + + // Next up, upsert our test data. + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/service.yaml")) + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/host.yaml")) + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/mapping1.yaml")) + + // Now we can check the IR. + checkIR(f, "initial", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(10, 10), + }) + + // Next up, upsert a completely unrelated mapping. This mustn't affect + // our existing group. + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/unrelated.yaml")) + + checkIR(f, "upsert unrelated", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(10, 10), + }) + + // Next, try updating the weight of workload2-mapping. + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/mapping2.yaml")) + + checkIR(f, "update workload2-mapping weight", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(50, 50), + }) + + // Next up, delete our completely unrelated mapping. This mustn't affect + // our existing group. + assert.NoError(t, f.Delete("Mapping", "infrastructure", "unrelated")) + + checkIR(f, "delete unrelated", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(50, 50), + }) + + // Repeat that upsert-and-repeat cycle. + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/unrelated.yaml")) + + checkIR(f, "re-upsert unrelated", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(50, 50), + }) + + assert.NoError(t, f.Delete("Mapping", "infrastructure", "unrelated")) + + checkIR(f, "re-delete unrelated", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(-1, 100), + "workload2-mapping": NewWeightCheck(50, 50), + }) + + // Finally, do something complex: update the weight of workload1-mapping, + // add a workload3-mapping, and reintroduce the unrelated mapping. + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/mapping3.yaml")) + assert.NoError(t, f.UpsertFile("testdata/unrelated-mappings/unrelated.yaml")) + + checkIR(f, "complex 1", map[string]WeightCheck{ + "workload1-mapping": NewWeightCheck(20, 20), + "workload2-mapping": NewWeightCheck(50, 70), + "workload3-mapping": NewWeightCheck(-1, 100), + }) +} diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml index 32da6bfed0..74e66bfb68 100644 --- a/docs/releaseNotes.yml +++ b/docs/releaseNotes.yml @@ -35,15 +35,15 @@ items: - version: 3.10.0 prevVersion: 3.9.0 date: 'TBD' - notes: + notes: - title: Upgrade to Envoy 1.28.0 type: feature body: >- - This upgrades $productName$ to be built on Envoy v1.28.0 which provides security, performance - and feature enhancements. You can read more about them here: + This upgrades $productName$ to be built on Envoy v1.28.0 which provides security, performance + and feature enhancements. You can read more about them here: Envoy Proxy 1.28.0 Release Notes docs: https://www.envoyproxy.io/docs/envoy/v1.28.0/version_history/version_history - + - title: Remove Ambassador Agent from published YAML Manifest type: change body: >- @@ -51,13 +51,22 @@ items: This is an optional component that provides additional features on top of $productName$ and we recommend installing it using the instructions found in the Ambassador Agent Repo. docs: https://github.com/datawire/ambassador-agent - + - title: Update to golang 1.21.5 type: change body: >- Upgraded $productName$ to the latest release of Golang as part of our general dependency upgrade process. - + - title: Fix internal keying for regex Mappings + type: bugfix + body: >- + $productName$ was incorrectly caching Mappings with regex headers + using the header name instead of the Mapping name, which could + reduce the cache's effectiveness. This has been fixed so that the + correct key is used. + github: + - title: "Incorrect Cache Key for Mapping" + link: https://github.com/emissary-ingress/emissary/issues/5714 - version: 3.9.0 prevVersion: 3.8.0 @@ -66,34 +75,34 @@ items: - title: Upgrade to Envoy 1.27.2 type: feature body: >- - This upgrades $productName$ to be built on Envoy v1.27.2 which provides security, performance - and feature enhancements. You can read more about them here: + This upgrades $productName$ to be built on Envoy v1.27.2 which provides security, performance + and feature enhancements. You can read more about them here: Envoy Proxy 1.27.2 Release Notes docs: https://www.envoyproxy.io/docs/envoy/v1.27.2/version_history/version_history - title: Added support for RESOURCE_EXHAUSTED responses to grpc clients when rate limited type: feature body: >- - By default, $productName$ will return an UNAVAILABLE code when a request using gRPC - is rate limited. The RateLimitService resource now exposes a new grpc.use_resource_exhausted_code - field that when set to true, $productName$ will return a RESOURCE_EXHAUSTED gRPC code instead. + By default, $productName$ will return an UNAVAILABLE code when a request using gRPC + is rate limited. The RateLimitService resource now exposes a new grpc.use_resource_exhausted_code + field that when set to true, $productName$ will return a RESOURCE_EXHAUSTED gRPC code instead. Thanks to Jerome Froelich for contributing this feature! - title: Added support for setting specific Envoy runtime flags in the Module type: feature body: >- - Envoy runtime fields that were provided to mitigate the recent HTTP/2 rapid reset vulnerability - can now be configured via the Module resource so the configuration will persist between restarts. - This configuration is added to the Envoy bootstrap config, so restarting Emissary is necessary after + Envoy runtime fields that were provided to mitigate the recent HTTP/2 rapid reset vulnerability + can now be configured via the Module resource so the configuration will persist between restarts. + This configuration is added to the Envoy bootstrap config, so restarting Emissary is necessary after changing these fields for the configuration to take effect. - title: Update APIExt minimum TLS version type: change body: >- - APIExt would previously allow for TLS 1.0 connections. We have updated it to now only use a minimum + APIExt would previously allow for TLS 1.0 connections. We have updated it to now only use a minimum TLS version of 1.3 to resolve security concerns. docs: https://www.tenable.com/plugins/nessus/104743 - + - title: Shipped Helm chart v8.9.0 type: change body: >- @@ -103,9 +112,9 @@ items: - title: Ensure APIExt server is available before starting Emissary-ingress type: bugfix body: >- - The APIExt server provides CRD conversion between the stored version v2 and the version watched for - by $productName$ v3alpha1. Since this component is required to operate $productName$, we have - introduced an init container that will ensure it is available before starting. This will help address + The APIExt server provides CRD conversion between the stored version v2 and the version watched for + by $productName$ v3alpha1. Since this component is required to operate $productName$, we have + introduced an init container that will ensure it is available before starting. This will help address some of the intermittent issues seen during install and upgrades. docs: https://artifacthub.io/packages/helm/datawire/edge-stack/$emissaryChartVersion$ diff --git a/python/ambassador/ir/ir.py b/python/ambassador/ir/ir.py index d0996a626d..bfb8b8a062 100644 --- a/python/ambassador/ir/ir.py +++ b/python/ambassador/ir/ir.py @@ -817,7 +817,13 @@ def add_mapping(self, aconf: Config, mapping: IRBaseMapping) -> Optional[IRBaseM else: self.logger.debug(f"IR: already have group for {mapping.name}") group = self.groups[mapping.group_id] - group.add_mapping(aconf, mapping) + + # Add mapping into the group only if the _cache_key doesn't exist in a group. + existing_mapping_cache_keys = [ group_mapping["_cache_key"] for group_mapping in group["mappings"] if "_cache_key" in group_mapping ] + if mapping["_cache_key"] in existing_mapping_cache_keys: + self.logger.debug(f"IR: _cache_key for {mapping.name} is {mapping['_cache_key']} already exists in a group.") + else: + group.add_mapping(aconf, mapping) self.cache_add(mapping) self.cache_add(group) diff --git a/python/ambassador/ir/irhttpmapping.py b/python/ambassador/ir/irhttpmapping.py index 700fe071ae..7eea649cc5 100644 --- a/python/ambassador/ir/irhttpmapping.py +++ b/python/ambassador/ir/irhttpmapping.py @@ -241,8 +241,8 @@ def __init__( if "regex_headers" in kwargs: # DON'T do anything special with a regex :authority match: we can't # do host-based filtering within the IR for it anyway. - for name, value in kwargs.get("regex_headers", {}).items(): - hdrs.append(KeyValueDecorator(name, value, regex=True)) + for hdr_name, hdr_value in kwargs.get("regex_headers", {}).items(): + hdrs.append(KeyValueDecorator(hdr_name, hdr_value, regex=True)) if "host" in kwargs: # It's deliberate that we'll allow kwargs['host'] to silently override an exact :authority diff --git a/python/ambassador_diag/diagd.py b/python/ambassador_diag/diagd.py index c93b30534f..316a49ade8 100644 --- a/python/ambassador_diag/diagd.py +++ b/python/ambassador_diag/diagd.py @@ -551,7 +551,7 @@ def check_cache(self) -> bool: result = False self.logger.error("CACHE: ENVOY CONFIG MISMATCH") errors += "econf diffs:\n" - errors += self.json_diff("econf", i1, i2) + errors += self.json_diff("econf", e1, e2) if not result: err_path = os.path.join(self.snapshot_path, "diff-tmp.txt")