From 325815598b5aee235dc0bbe847057de85d529d11 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 23 Feb 2024 10:21:30 +0800 Subject: [PATCH 1/2] move provider/uitls outside Signed-off-by: huabing zhao --- internal/cmd/egctl/config.go | 4 +++- internal/cmd/egctl/version.go | 7 +++---- internal/gatewayapi/backendtrafficpolicy.go | 6 ++---- internal/gatewayapi/clienttrafficpolicy.go | 6 ++---- internal/gatewayapi/helpers.go | 3 ++- internal/gatewayapi/listener.go | 4 ++-- internal/gatewayapi/runner/runner.go | 2 +- internal/gatewayapi/securitypolicy.go | 14 ++++---------- .../testdata/securitypolicy-with-oidc.out.yaml | 4 ++-- .../kubernetes/infra_resource.go | 6 ++---- .../kubernetes/proxy/resource.go | 6 +++--- .../kubernetes/proxy_serviceaccount_test.go | 4 ++-- internal/provider/kubernetes/controller.go | 2 +- .../provider/kubernetes/kubernetes_test.go | 17 ++++------------- internal/provider/kubernetes/predicates.go | 2 +- internal/provider/kubernetes/routes.go | 2 +- internal/provider/kubernetes/routes_test.go | 2 +- internal/provider/kubernetes/secrets.go | 7 ++----- internal/provider/kubernetes/status.go | 2 +- .../{provider/utils/utils.go => utils/misc.go} | 18 ++++++++++-------- .../utils/utils_test.go => utils/misc_test.go} | 6 +++--- .../translator/testdata/in/xds-ir/oidc.yaml | 2 +- .../testdata/out/xds-ir/oidc.listeners.yaml | 10 +++++----- 23 files changed, 58 insertions(+), 78 deletions(-) rename internal/{provider/utils/utils.go => utils/misc.go} (73%) rename internal/{provider/utils/utils_test.go => utils/misc_test.go} (88%) diff --git a/internal/cmd/egctl/config.go b/internal/cmd/egctl/config.go index 83fafa69c70..ca8a617473e 100644 --- a/internal/cmd/egctl/config.go +++ b/internal/cmd/egctl/config.go @@ -25,6 +25,7 @@ import ( "github.com/envoyproxy/gateway/internal/cmd/options" "github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/proxy" kube "github.com/envoyproxy/gateway/internal/kubernetes" + "github.com/envoyproxy/gateway/internal/utils" ) var ( @@ -168,7 +169,8 @@ func fetchRunningEnvoyPods(c kube.CLIClient, nn types.NamespacedName, labelSelec podsNamespacedNames := []types.NamespacedName{} for _, pod := range pods { - podNsName := types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name} + pod := pod + podNsName := utils.NamespacedName(&pod) if pod.Status.Phase != "Running" { return podsNamespacedNames, fmt.Errorf("pod %s is not running", podNsName) } diff --git a/internal/cmd/egctl/version.go b/internal/cmd/egctl/version.go index de3cde37a61..5cab0035358 100644 --- a/internal/cmd/egctl/version.go +++ b/internal/cmd/egctl/version.go @@ -20,6 +20,7 @@ import ( "github.com/envoyproxy/gateway/internal/cmd/options" "github.com/envoyproxy/gateway/internal/cmd/version" kube "github.com/envoyproxy/gateway/internal/kubernetes" + "github.com/envoyproxy/gateway/internal/utils" ) const ( @@ -98,16 +99,14 @@ func versions(w io.Writer, containerName, output string, remote bool) error { } for _, pod := range pods.Items { + pod := pod if pod.Status.Phase != "Running" { fmt.Fprintf(w, "WARN: pod %s/%s is not running, skipping it.", pod.Namespace, pod.Name) continue } - nn := types.NamespacedName{ - Namespace: pod.Namespace, - Name: pod.Name, - } + nn := utils.NamespacedName(&pod) stdout, _, err := c.PodExec(nn, containerName, "envoy-gateway version -ojson") if err != nil { return fmt.Errorf("pod exec on %s/%s failed: %w", nn.Namespace, nn.Name, err) diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index d5d71b1fd38..182c5d5c2dd 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -23,6 +23,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/utils/regex" ) @@ -66,10 +67,7 @@ func (t *Translator) ProcessBackendTrafficPolicies(backendTrafficPolicies []*egv } gatewayMap := map[types.NamespacedName]*policyGatewayTargetContext{} for _, gw := range gateways { - key := types.NamespacedName{ - Name: gw.GetName(), - Namespace: gw.GetNamespace(), - } + key := utils.NamespacedName(gw) gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw} } diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index 5bd81cabf95..f5992db21a8 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -21,6 +21,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils" ) const ( @@ -64,10 +65,7 @@ func (t *Translator) ProcessClientTrafficPolicies(resources *Resources, } // Check for conflicts - key := types.NamespacedName{ - Name: gateway.Name, - Namespace: gateway.Namespace, - } + key := utils.NamespacedName(gateway) // Check if another policy targeting the same section exists section := string(*(policy.Spec.TargetRef.SectionName)) diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go index 4c4ef61aece..fd5456191ac 100644 --- a/internal/gatewayapi/helpers.go +++ b/internal/gatewayapi/helpers.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils" ) const ( @@ -126,7 +127,7 @@ func GetReferencedListeners(parentRef gwapiv1.ParentReference, gateways []*Gatew var referencedListeners []*ListenerContext for _, gateway := range gateways { - if !IsRefToGateway(parentRef, types.NamespacedName{Namespace: gateway.Namespace, Name: gateway.Name}) { + if !IsRefToGateway(parentRef, utils.NamespacedName(gateway)) { continue } diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index 1a92c751dea..4702d2a4e26 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -9,11 +9,11 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/utils/naming" ) @@ -242,7 +242,7 @@ func processTracing(gw *gwapiv1.Gateway, envoyproxy *egv1a1.EnvoyProxy) *ir.Trac } return &ir.Tracing{ - ServiceName: naming.ServiceName(types.NamespacedName{Name: gw.Name, Namespace: gw.Namespace}), + ServiceName: naming.ServiceName(utils.NamespacedName(gw)), ProxyTracing: *envoyproxy.Spec.Telemetry.Tracing, } } diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index ac2ec3140a0..ebc5d612d25 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -16,7 +16,7 @@ import ( extension "github.com/envoyproxy/gateway/internal/extension/types" "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/message" - "github.com/envoyproxy/gateway/internal/provider/utils" + "github.com/envoyproxy/gateway/internal/utils" ) type Config struct { diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 0e5d91145e5..e2121cdfcab 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "fmt" - "hash/fnv" "net/http" "net/netip" "net/url" @@ -28,6 +27,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils" ) const ( @@ -61,10 +61,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security } gatewayMap := map[types.NamespacedName]*policyGatewayTargetContext{} for _, gw := range gateways { - key := types.NamespacedName{ - Name: gw.GetName(), - Namespace: gw.GetNamespace(), - } + key := utils.NamespacedName(gw) gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw} } @@ -495,11 +492,8 @@ func (t *Translator) buildOIDC( logoutPath = *oidc.LogoutPath } - h := fnv.New32a() - if _, err = h.Write([]byte(policy.UID)); err != nil { - return nil, fmt.Errorf("error generating oauth cookie suffix: %w", err) - } - suffix := fmt.Sprintf("%X", h.Sum32()) + // Generate a unique cookie suffix for oauth filters + suffix := utils.Digest(string(policy.UID)) return &ir.OIDC{ Provider: *provider, diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml index c08304d3ebd..30b5971ece3 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml @@ -232,7 +232,7 @@ xdsIR: oidc: clientID: client2.oauth.foo.com clientSecret: Y2xpZW50MTpzZWNyZXQK - cookieSuffix: 5F93C2E4 + cookieSuffix: 5f93c2e4 logoutPath: /foo/logout provider: authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth @@ -264,7 +264,7 @@ xdsIR: oidc: clientID: client1.apps.googleusercontent.com clientSecret: Y2xpZW50MTpzZWNyZXQK - cookieSuffix: B0A1B740 + cookieSuffix: b0a1b740 logoutPath: /bar/logout provider: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth diff --git a/internal/infrastructure/kubernetes/infra_resource.go b/internal/infrastructure/kubernetes/infra_resource.go index d0e21be628e..e03f33ce788 100644 --- a/internal/infrastructure/kubernetes/infra_resource.go +++ b/internal/infrastructure/kubernetes/infra_resource.go @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource" + "github.com/envoyproxy/gateway/internal/utils" ) // createOrUpdateServiceAccount creates a ServiceAccount in the kube api server based on the @@ -29,10 +30,7 @@ func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRend } current := &corev1.ServiceAccount{} - key := types.NamespacedName{ - Namespace: sa.Namespace, - Name: sa.Name, - } + key := utils.NamespacedName(sa) return i.Client.CreateOrUpdate(ctx, key, current, sa, func() bool { // the service account never changed, does not need to update diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go index 7bedbd9cb23..fc9751fcec3 100644 --- a/internal/infrastructure/kubernetes/proxy/resource.go +++ b/internal/infrastructure/kubernetes/proxy/resource.go @@ -16,7 +16,7 @@ import ( "github.com/envoyproxy/gateway/internal/envoygateway/config" "github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource" "github.com/envoyproxy/gateway/internal/ir" - providerutils "github.com/envoyproxy/gateway/internal/provider/utils" + "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/xds/bootstrap" ) @@ -52,7 +52,7 @@ var ( // ExpectedResourceHashedName returns expected resource hashed name including up to the 48 characters of the original name. func ExpectedResourceHashedName(name string) string { - hashedName := providerutils.GetHashedName(name, 48) + hashedName := utils.GetHashedName(name, 48) return fmt.Sprintf("%s-%s", config.EnvoyPrefix, hashedName) } @@ -116,7 +116,7 @@ func expectedProxyContainers(infra *ir.ProxyInfra, } port := corev1.ContainerPort{ // hashed container port name including up to the 6 characters of the port name and the maximum of 15 characters. - Name: providerutils.GetHashedName(p.Name, 6), + Name: utils.GetHashedName(p.Name, 6), ContainerPort: p.ContainerPort, Protocol: protocol, } diff --git a/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go b/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go index 0cde399a2df..49f7c798372 100644 --- a/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go +++ b/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go @@ -54,7 +54,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Namespace: "test", - Name: "envoy-test-9f86d081", + Name: "envoy-test-afd071e5", Labels: map[string]string{ "app.kubernetes.io/name": "envoy", "app.kubernetes.io/component": "proxy", @@ -103,7 +103,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Namespace: "test", - Name: "envoy-test-9f86d081", + Name: "envoy-test-afd071e5", Labels: map[string]string{ "app.kubernetes.io/name": "envoy", "app.kubernetes.io/component": "proxy", diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index af42d4c178b..b7d305caa83 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -37,8 +37,8 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/logging" "github.com/envoyproxy/gateway/internal/message" - "github.com/envoyproxy/gateway/internal/provider/utils" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/utils/slice" ) diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 2c470f12d02..301b7d392c2 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -321,7 +321,7 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro // Ensure the Gateway reports "Scheduled". require.Eventually(t, func() bool { - if err := cli.Get(ctx, types.NamespacedName{Namespace: gw.Namespace, Name: gw.Name}, gw); err != nil { + if err := cli.Get(ctx, utils.NamespacedName(gw), gw); err != nil { return false } @@ -354,10 +354,7 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro }, defaultWait, defaultTick) // Ensure the test Gateway in the Gateway resources is as expected. - key := types.NamespacedName{ - Namespace: gw.Namespace, - Name: gw.Name, - } + key := utils.NamespacedName(gw) require.Eventually(t, func() bool { return cli.Get(ctx, key, gw) == nil }, defaultWait, defaultTick) @@ -884,10 +881,7 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, defaultWait, defaultTick) // Ensure the test HTTPRoute in the HTTPRoute resources is as expected. - key := types.NamespacedName{ - Namespace: testCase.route.Namespace, - Name: testCase.route.Name, - } + key := utils.NamespacedName(testCase.route) require.Eventually(t, func() bool { return cli.Get(ctx, key, &testCase.route) == nil }, defaultWait, defaultTick) @@ -1035,10 +1029,7 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc }, defaultWait, defaultTick) // Ensure the test TLSRoute in the TLSRoute resources is as expected. - key := types.NamespacedName{ - Namespace: testCase.route.Namespace, - Name: testCase.route.Name, - } + key := utils.NamespacedName(testCase.route) require.Eventually(t, func() bool { return cli.Get(ctx, key, &testCase.route) == nil }, defaultWait, defaultTick) diff --git a/internal/provider/kubernetes/predicates.go b/internal/provider/kubernetes/predicates.go index 5a042d1cacf..52f72a445f5 100644 --- a/internal/provider/kubernetes/predicates.go +++ b/internal/provider/kubernetes/predicates.go @@ -23,7 +23,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/gatewayapi" - "github.com/envoyproxy/gateway/internal/provider/utils" + "github.com/envoyproxy/gateway/internal/utils" ) // hasMatchingController returns true if the provided object is a GatewayClass diff --git a/internal/provider/kubernetes/routes.go b/internal/provider/kubernetes/routes.go index ae0fa3fe883..c3c02ec72c5 100644 --- a/internal/provider/kubernetes/routes.go +++ b/internal/provider/kubernetes/routes.go @@ -17,7 +17,7 @@ import ( gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/envoyproxy/gateway/internal/gatewayapi" - "github.com/envoyproxy/gateway/internal/provider/utils" + "github.com/envoyproxy/gateway/internal/utils" ) // processTLSRoutes finds TLSRoutes corresponding to a gatewayNamespaceName, further checks for diff --git a/internal/provider/kubernetes/routes_test.go b/internal/provider/kubernetes/routes_test.go index b7d747aa3e8..1c99b654f03 100644 --- a/internal/provider/kubernetes/routes_test.go +++ b/internal/provider/kubernetes/routes_test.go @@ -26,7 +26,7 @@ import ( "github.com/envoyproxy/gateway/internal/envoygateway" "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/logging" - "github.com/envoyproxy/gateway/internal/provider/utils" + "github.com/envoyproxy/gateway/internal/utils" ) func TestProcessHTTPRoutes(t *testing.T) { diff --git a/internal/provider/kubernetes/secrets.go b/internal/provider/kubernetes/secrets.go index e14f67b7ccb..5a2a8c7776c 100644 --- a/internal/provider/kubernetes/secrets.go +++ b/internal/provider/kubernetes/secrets.go @@ -14,10 +14,10 @@ import ( corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/envoyproxy/gateway/internal/crypto" + "github.com/envoyproxy/gateway/internal/utils" ) var ( @@ -86,10 +86,7 @@ func CreateOrUpdateSecrets(ctx context.Context, client client.Client, secrets [] for i := range secrets { secret := secrets[i] current := new(corev1.Secret) - key := types.NamespacedName{ - Namespace: secret.Namespace, - Name: secret.Name, - } + key := utils.NamespacedName(&secret) if err := client.Get(ctx, key, current); err != nil { // Create if not found. if kerrors.IsNotFound(err) { diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index 83f88579c46..f26ae4c877a 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -17,8 +17,8 @@ import ( "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/message" - "github.com/envoyproxy/gateway/internal/provider/utils" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils" ) // subscribeAndUpdateStatus subscribes to gateway API object status updates and diff --git a/internal/provider/utils/utils.go b/internal/utils/misc.go similarity index 73% rename from internal/provider/utils/utils.go rename to internal/utils/misc.go index 48f3f8703f6..9f975ea7e87 100644 --- a/internal/provider/utils/utils.go +++ b/internal/utils/misc.go @@ -6,8 +6,8 @@ package utils import ( - "crypto/sha256" "fmt" + "hash/fnv" "strings" "k8s.io/apimachinery/pkg/types" @@ -25,19 +25,21 @@ func NamespacedName(obj client.Object) types.NamespacedName { // GetHashedName returns a partially hashed name for the string including up to the given length of the original name characters before the hash. // Input `nsName` should be formatted as `{Namespace}/{ResourceName}`. func GetHashedName(nsName string, length int) string { - hashedName := HashString(nsName) + hashedName := Digest(nsName) // replace `/` with `-` to create a valid K8s resource name resourceName := strings.ReplaceAll(nsName, "/", "-") if length > 0 && len(resourceName) > length { // resource name needs to be trimmed, as container port name must not contain consecutive hyphens trimmedName := strings.TrimSuffix(resourceName[0:length], "-") - return fmt.Sprintf("%s-%s", trimmedName, hashedName[0:8]) + return fmt.Sprintf("%s-%s", trimmedName, hashedName) } - return fmt.Sprintf("%s-%s", resourceName, hashedName[0:8]) + return fmt.Sprintf("%s-%s", resourceName, hashedName) } -func HashString(str string) string { - h := sha256.New() // Using sha256 instead of sha1 due to Blocklisted import crypto/sha1: weak cryptographic primitive (gosec) - h.Write([]byte(str)) - return strings.ToLower(fmt.Sprintf("%x", h.Sum(nil))) +// Digest returns a 32-bit hashh of the input string. +// The hash is represented as a capitalized hexadecimal string. +func Digest(str string) string { + h := fnv.New32a() + _, _ = h.Write([]byte(str)) + return fmt.Sprintf("%x", h.Sum32()) } diff --git a/internal/provider/utils/utils_test.go b/internal/utils/misc_test.go similarity index 88% rename from internal/provider/utils/utils_test.go rename to internal/utils/misc_test.go index 5955f28aecd..43967813867 100644 --- a/internal/provider/utils/utils_test.go +++ b/internal/utils/misc_test.go @@ -18,9 +18,9 @@ func TestGetHashedName(t *testing.T) { length int expected string }{ - {"test default name", "http", 6, "http-e0603c49"}, - {"test removing trailing slash", "namespace/name", 10, "namespace-18a6500f"}, - {"test removing trailing hyphen", "envoy-gateway-system/eg/http", 6, "envoy-2ecf157b"}, + {"test default name", "http", 6, "http-c96448a5"}, + {"test removing trailing slash", "namespace/name", 10, "namespace-3c4f601e"}, + {"test removing trailing hyphen", "envoy-gateway-system/eg/http", 6, "envoy-128ffda5"}, } for _, tc := range testCases { diff --git a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml index ee69934f1c5..b90a1c97f4a 100644 --- a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml @@ -55,4 +55,4 @@ http: redirectURL: "https://www.example.com/bar/oauth2/callback" redirectPath: "/bar/oauth2/callback" logoutPath: "/bar/logout" - cookieSuffix: B0A1B740 + cookieSuffix: 5f93c2e4 diff --git a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml index abadd4dd23c..8173e9f9e29 100644 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml @@ -69,11 +69,11 @@ credentials: clientId: client.oauth.bar.com cookieNames: - bearerToken: BearerToken-B0A1B740 - idToken: IdToken-B0A1B740 - oauthExpires: OauthExpires-B0A1B740 - oauthHmac: OauthHMAC-B0A1B740 - refreshToken: RefreshToken-B0A1B740 + bearerToken: BearerToken-5f93c2e4 + idToken: IdToken-5f93c2e4 + oauthExpires: OauthExpires-5f93c2e4 + oauthHmac: OauthHMAC-5f93c2e4 + refreshToken: RefreshToken-5f93c2e4 hmacSecret: name: second-route/oauth2/hmac_secret sdsConfig: From 1c3657394050433af00675ec97c0b658578ea5e1 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 23 Feb 2024 11:20:18 +0800 Subject: [PATCH 2/2] keep the original HashString for backward compatibility Signed-off-by: huabing zhao --- internal/gatewayapi/securitypolicy.go | 2 +- .../kubernetes/proxy_serviceaccount_test.go | 4 ++-- .../provider/kubernetes/kubernetes_test.go | 5 ++-- internal/utils/misc.go | 23 ++++++++++++++----- internal/utils/misc_test.go | 6 ++--- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index e2121cdfcab..abb274e6b08 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -493,7 +493,7 @@ func (t *Translator) buildOIDC( } // Generate a unique cookie suffix for oauth filters - suffix := utils.Digest(string(policy.UID)) + suffix := utils.Digest32(string(policy.UID)) return &ir.OIDC{ Provider: *provider, diff --git a/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go b/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go index 49f7c798372..0cde399a2df 100644 --- a/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go +++ b/internal/infrastructure/kubernetes/proxy_serviceaccount_test.go @@ -54,7 +54,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Namespace: "test", - Name: "envoy-test-afd071e5", + Name: "envoy-test-9f86d081", Labels: map[string]string{ "app.kubernetes.io/name": "envoy", "app.kubernetes.io/component": "proxy", @@ -103,7 +103,7 @@ func TestCreateOrUpdateProxyServiceAccount(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{ Namespace: "test", - Name: "envoy-test-afd071e5", + Name: "envoy-test-9f86d081", Labels: map[string]string{ "app.kubernetes.io/name": "envoy", "app.kubernetes.io/component": "proxy", diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 301b7d392c2..cc28f0d8497 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -37,6 +37,7 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/message" "github.com/envoyproxy/gateway/internal/provider/kubernetes/test" + "github.com/envoyproxy/gateway/internal/utils" ) const ( @@ -881,7 +882,7 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour }, defaultWait, defaultTick) // Ensure the test HTTPRoute in the HTTPRoute resources is as expected. - key := utils.NamespacedName(testCase.route) + key := utils.NamespacedName(&testCase.route) require.Eventually(t, func() bool { return cli.Get(ctx, key, &testCase.route) == nil }, defaultWait, defaultTick) @@ -1029,7 +1030,7 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc }, defaultWait, defaultTick) // Ensure the test TLSRoute in the TLSRoute resources is as expected. - key := utils.NamespacedName(testCase.route) + key := utils.NamespacedName(&testCase.route) require.Eventually(t, func() bool { return cli.Get(ctx, key, &testCase.route) == nil }, defaultWait, defaultTick) diff --git a/internal/utils/misc.go b/internal/utils/misc.go index 9f975ea7e87..6e434660de6 100644 --- a/internal/utils/misc.go +++ b/internal/utils/misc.go @@ -6,6 +6,7 @@ package utils import ( + "crypto/sha256" "fmt" "hash/fnv" "strings" @@ -25,20 +26,30 @@ func NamespacedName(obj client.Object) types.NamespacedName { // GetHashedName returns a partially hashed name for the string including up to the given length of the original name characters before the hash. // Input `nsName` should be formatted as `{Namespace}/{ResourceName}`. func GetHashedName(nsName string, length int) string { - hashedName := Digest(nsName) + hashedName := Digest256(nsName) // replace `/` with `-` to create a valid K8s resource name resourceName := strings.ReplaceAll(nsName, "/", "-") if length > 0 && len(resourceName) > length { // resource name needs to be trimmed, as container port name must not contain consecutive hyphens trimmedName := strings.TrimSuffix(resourceName[0:length], "-") - return fmt.Sprintf("%s-%s", trimmedName, hashedName) + return fmt.Sprintf("%s-%s", trimmedName, hashedName[0:8]) } - return fmt.Sprintf("%s-%s", resourceName, hashedName) + // Ideally we should use 32-bit hash instead of 64-bit hash and return the first 8 characters of the hash. + // However, we are using 64-bit hash to maintain backward compatibility. + return fmt.Sprintf("%s-%s", resourceName, hashedName[0:8]) } -// Digest returns a 32-bit hashh of the input string. -// The hash is represented as a capitalized hexadecimal string. -func Digest(str string) string { +// Digest256 returns a sha256 hash of the input string. +// The hash is represented as a hexadecimal string of length 64. +func Digest256(str string) string { + h := sha256.New() // Using sha256 instead of sha1 due to Blocklisted import crypto/sha1: weak cryptographic primitive (gosec) + h.Write([]byte(str)) + return strings.ToLower(fmt.Sprintf("%x", h.Sum(nil))) +} + +// Digest32 returns a 32-bit hash of the input string. +// The hash is represented as a hexadecimal string of length 8. +func Digest32(str string) string { h := fnv.New32a() _, _ = h.Write([]byte(str)) return fmt.Sprintf("%x", h.Sum32()) diff --git a/internal/utils/misc_test.go b/internal/utils/misc_test.go index 43967813867..5955f28aecd 100644 --- a/internal/utils/misc_test.go +++ b/internal/utils/misc_test.go @@ -18,9 +18,9 @@ func TestGetHashedName(t *testing.T) { length int expected string }{ - {"test default name", "http", 6, "http-c96448a5"}, - {"test removing trailing slash", "namespace/name", 10, "namespace-3c4f601e"}, - {"test removing trailing hyphen", "envoy-gateway-system/eg/http", 6, "envoy-128ffda5"}, + {"test default name", "http", 6, "http-e0603c49"}, + {"test removing trailing slash", "namespace/name", 10, "namespace-18a6500f"}, + {"test removing trailing hyphen", "envoy-gateway-system/eg/http", 6, "envoy-2ecf157b"}, } for _, tc := range testCases {