From 15a5b0b40826f6b5ea966c43fbadad98bfa4f388 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 14:10:34 +0100 Subject: [PATCH 01/14] refactor: envoyfilters as types Signed-off-by: Lukas Hoehl --- go.mod | 26 +- go.sum | 20 +- pkg/controller/actuator.go | 5 +- pkg/envoyfilters/envoyfilters.go | 461 ++++++++++++++++++++----------- pkg/webhook/webhook.go | 172 ++++++++++++ 5 files changed, 501 insertions(+), 183 deletions(-) create mode 100644 pkg/webhook/webhook.go diff --git a/go.mod b/go.mod index 1094823d5..702b39309 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,10 @@ toolchain go1.24.6 require ( github.com/ahmetb/gen-crd-api-reference-docs v0.3.0 - github.com/gardener/gardener v1.117.6 - github.com/gardener/gardener-extension-provider-openstack v1.47.0 - github.com/go-logr/logr v1.4.3 + github.com/envoyproxy/go-control-plane/envoy v1.32.3 + github.com/gardener/gardener v1.112.0 + github.com/gardener/gardener-extension-provider-openstack v1.41.2 + github.com/go-logr/logr v1.4.2 github.com/golang/mock v1.6.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 @@ -25,12 +26,15 @@ require ( k8s.io/client-go v0.32.7 k8s.io/code-generator v0.32.7 k8s.io/component-base v0.32.7 + gomodules.xyz/jsonpatch/v2 v2.4.0 + google.golang.org/protobuf v1.36.5 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 sigs.k8s.io/controller-runtime v0.20.4 sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231015215740-bf15e44028f9 ) require ( + cel.dev/expr v0.16.1 // indirect dario.cat/mergo v1.0.1 // indirect github.com/BurntSushi/toml v1.4.0 // indirect github.com/Masterminds/goutils v1.1.1 // indirect @@ -39,11 +43,13 @@ require ( github.com/andybalholm/brotli v1.1.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78 // indirect github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/cyphar/filepath-securejoin v0.3.6 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect - github.com/evanphx/json-patch/v5 v5.9.11 // indirect + github.com/envoyproxy/protoc-gen-validate v1.1.0 // indirect + github.com/evanphx/json-patch/v5 v5.9.0 // indirect github.com/fatih/color v1.18.0 // indirect github.com/fluent/fluent-operator/v3 v3.3.0 // indirect github.com/fsnotify/fsnotify v1.8.0 // indirect @@ -87,15 +93,16 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect - github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.81.0 // indirect - github.com/prometheus/client_golang v1.22.0 // indirect + github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect + github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.78.2 // indirect + github.com/prometheus/client_golang v1.20.5 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.63.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/shopspring/decimal v1.4.0 // indirect - github.com/spf13/afero v1.14.0 // indirect - github.com/spf13/cast v1.7.1 // indirect + github.com/spf13/afero v1.12.0 // indirect + github.com/spf13/cast v1.7.0 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect @@ -116,8 +123,7 @@ require ( golang.org/x/time v0.11.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect - google.golang.org/protobuf v1.36.5 // indirect - gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect helm.sh/helm/v3 v3.17.3 // indirect diff --git a/go.sum b/go.sum index 19e956871..beb4d37b2 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +cel.dev/expr v0.16.1 h1:NR0+oFYzR1CqLFhTAqg3ql59G9VfN8fKq1TCHJ6gq1g= +cel.dev/expr v0.16.1/go.mod h1:AsGA5zb3WruAEQeQng1RZdGEXmBj0jvMWh6l5SnNuC8= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= @@ -58,6 +60,8 @@ github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWR github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= +github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78 h1:QVw89YDxXxEe+l8gU8ETbOasdwEV+avkR75ZzsVV9WI= +github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= @@ -76,7 +80,11 @@ github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= +github.com/envoyproxy/go-control-plane/envoy v1.32.3 h1:hVEaommgvzTjTd4xCaFd+kEQ2iYBtGxP6luyLrx6uOk= +github.com/envoyproxy/go-control-plane/envoy v1.32.3/go.mod h1:F6hWupPfh75TBXGKA++MCT/CZHFq5r9/uwt/kQYkZfE= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= +github.com/envoyproxy/protoc-gen-validate v1.1.0 h1:tntQDh69XqOCOZsDz0lVJQez/2L6Uu2PdjCQwWCJ3bM= +github.com/envoyproxy/protoc-gen-validate v1.1.0/go.mod h1:sXRDRVmzEbkM7CVcM06s9shE/m23dg3wzjl0UWqJ2q4= github.com/evanphx/json-patch v4.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls= github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= @@ -285,6 +293,8 @@ github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -531,6 +541,8 @@ google.golang.org/genproto v0.0.0-20191230161307-f3c370f40bfb/go.mod h1:n3cpQtvx google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 h1:CkkIfIt50+lT6NHAVoRYEyAvQGFM7xEwXUUywFvEb3Q= google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576/go.mod h1:1R3kvZ1dtP3+4p4d3G8uJ8rFk/fWlScl38vanWACI08= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8 h1:TqExAhdPaB60Ux47Cn0oLV07rGnxZzIsaRhQaqS666A= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8/go.mod h1:lcTa1sDdWEIHMWlITnIczmw5w60CF9ffkb8Z+DVmmjA= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= @@ -576,10 +588,10 @@ honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= -istio.io/api v1.25.2 h1:FCRQy7iaTreKJdLemlQ1vRJEsf1soCHoTAuSf68w5I8= -istio.io/api v1.25.2/go.mod h1:QFzEXv/IT582T0FHZVp1QoolvE4ws0zz/vVO55blmlE= -istio.io/client-go v1.25.1 h1:1Asibz5v2NBA6w4Sqa85NS7TkpEolZcPKAT5oUAXWi8= -istio.io/client-go v1.25.1/go.mod h1:Vap9OyHJMvvDegYoZczcNybS4wbPaTk+4bZcWMb8+vE= +istio.io/api v1.25.0 h1:UVQNFUAz4R06NagA+QpEzLmi4sw+m+UuMgD6sIyXawA= +istio.io/api v1.25.0/go.mod h1:QFzEXv/IT582T0FHZVp1QoolvE4ws0zz/vVO55blmlE= +istio.io/client-go v1.24.2 h1:JTTfBV6dv+AAW+AfccyrdX4T1f9CpsXd1Yzo1s/IYAI= +istio.io/client-go v1.24.2/go.mod h1:dgZ9EmJzh1EECzf6nQhwNL4R6RvlyeH/RXeNeNp/MRg= k8s.io/api v0.19.0/go.mod h1:I1K45XlvTrDjmj5LoM5LuP/KYrhWbjUKT/SoPG0qTjw= k8s.io/api v0.32.7 h1:CBhHkoi3YJW8QQI6VL/Hu9f1HHVImmuIh513d4H4VfQ= k8s.io/api v0.32.7/go.mod h1:YEB46LZ/M0/9t0m+R2FxW5fkZAUR/eoS6sZQKS3mBYk= diff --git a/pkg/controller/actuator.go b/pkg/controller/actuator.go index 94f73194a..9fe94b1f2 100644 --- a/pkg/controller/actuator.go +++ b/pkg/controller/actuator.go @@ -276,12 +276,9 @@ func (a *actuator) createSeedResources( return err } - vpnEnvoyFilterSpec, err := envoyfilters.BuildVPNEnvoyFilterSpecForHelmChart( + vpnEnvoyFilterSpec := envoyfilters.BuildVPNEnvoyFilterSpecForHelmChart( cluster, spec.Rule, alwaysAllowedCIDRs, istioLabels, ) - if err != nil { - return err - } cfg := map[string]interface{}{ "shootName": cluster.Shoot.Status.TechnicalID, diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 79619d6a3..33bbf5491 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -2,10 +2,22 @@ package envoyfilters import ( "errors" + "fmt" "net" "strings" + envoy_corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + envoyconfig_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" + envoy_routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + envoyhttp_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3" + envoynetwork_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3" + matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" "github.com/gardener/gardener/extensions/pkg/controller" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/structpb" + "google.golang.org/protobuf/types/known/wrapperspb" + istio_networkingv1alpha3 "istio.io/api/networking/v1alpha3" "github.com/stackitcloud/gardener-extension-acl/pkg/helper" ) @@ -26,21 +38,56 @@ type ACLRule struct { Type string `json:"type"` } +func (r *ACLRule) actionProto() envoyconfig_rbacv3.RBAC_Action { + switch r.Action { + case "DENY": + return envoyconfig_rbacv3.RBAC_DENY + case "ALLOW": + return envoyconfig_rbacv3.RBAC_ALLOW + default: + panic("unknown action") + } +} + +// FilterPatch represents the object beneath EnvoyFilter.spec.configPatches.patch.value +// It holds the name of the filter and it's typed config to inject into the envoy config +type FilterPatch struct { + Name string `json:"name"` + TypedConfig *structpb.Struct `json:"typed_config"` +} + +// AsStructPB returns FilterPatch represented as a structpb.Struct +func (f *FilterPatch) asStructPB() *structpb.Struct { + pb, err := structpb.NewStruct(map[string]any{ + "name": f.Name, + "typed_config": f.TypedConfig.AsMap(), + }) + if err != nil { + // This state is not valid and should not be propergated + panic(err) + } + return pb +} + +// AsMap returns FilterPatch represented as a map[string]interface{} +func (f *FilterPatch) AsMap() map[string]interface{} { + return f.asStructPB().AsMap() +} + // BuildAPIEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for API server // networking for every rule in the extension spec. func BuildAPIEnvoyFilterSpecForHelmChart( rule *ACLRule, hosts, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) (map[string]interface{}, error) { +) (*istio_networkingv1alpha3.EnvoyFilter, error) { apiConfigPatch, err := CreateAPIConfigPatchFromRule(rule, hosts, alwaysAllowedCIDRs) if err != nil { return nil, err } - - return map[string]interface{}{ - "workloadSelector": map[string]interface{}{ - "labels": istioLabels, + return &istio_networkingv1alpha3.EnvoyFilter{ + WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ + Labels: istioLabels, }, - "configPatches": []map[string]interface{}{ + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ apiConfigPatch, }, }, nil @@ -50,17 +97,17 @@ func BuildAPIEnvoyFilterSpecForHelmChart( // endpoints using the seed ingress domain. func BuildIngressEnvoyFilterSpecForHelmChart( cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) map[string]interface{} { +) *istio_networkingv1alpha3.EnvoyFilter { seedIngressDomain := helper.GetSeedIngressDomain(cluster.Seed) if seedIngressDomain != "" { shootID := helper.ComputeShortShootID(cluster.Shoot) - return map[string]interface{}{ - "workloadSelector": map[string]interface{}{ - "labels": istioLabels, + return &istio_networkingv1alpha3.EnvoyFilter{ + WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ + Labels: istioLabels, }, - "configPatches": []map[string]interface{}{ - CreateIngressConfigPatchFromRule(rule, seedIngressDomain, shootID, alwaysAllowedCIDRs), + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + ingressConfigPatchFromRule(rule, seedIngressDomain, shootID, alwaysAllowedCIDRs), }, } } @@ -70,20 +117,15 @@ func BuildIngressEnvoyFilterSpecForHelmChart( // BuildVPNEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for VPN. func BuildVPNEnvoyFilterSpecForHelmChart( cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) (map[string]interface{}, error) { - vpnConfigPatch, err := CreateVPNConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs) - if err != nil { - return nil, err - } - - return map[string]interface{}{ - "workloadSelector": map[string]interface{}{ - "labels": istioLabels, +) *istio_networkingv1alpha3.EnvoyFilter { + return &istio_networkingv1alpha3.EnvoyFilter{ + WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ + Labels: istioLabels, }, - "configPatches": []map[string]interface{}{ - vpnConfigPatch, + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + vpnConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs), }, - }, nil + } } // CreateAPIConfigPatchFromRule combines an ACLRule, the first entry of the @@ -91,153 +133,200 @@ func BuildVPNEnvoyFilterSpecForHelmChart( // applied to the `GATEWAY` network filter chain matching the host. func CreateAPIConfigPatchFromRule( rule *ACLRule, hosts, alwaysAllowedCIDRs []string, -) (map[string]interface{}, error) { +) (*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { if len(hosts) == 0 { return nil, ErrNoHostsGiven } rbacName := "acl-api" principals := ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs) - return map[string]interface{}{ - "applyTo": "NETWORK_FILTER", - "match": map[string]interface{}{ - "context": "GATEWAY", - "listener": map[string]interface{}{ - "filterChain": map[string]interface{}{ - // There is one filter chain per shoot in the SNI listener that has two SNI matches: one for the internal and - // one for the external shoot domain. - // We can use either shoot domain to match the filter chain that we want to patch with this EnvoyFilter. - // The ACL config will apply to traffic going via both the internal and the external API server address. - // See: https://istio.io/latest/docs/reference/config/networking/envoy-filter/#EnvoyFilter-ListenerMatch-FilterChainMatch - "sni": hosts[0], + return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, + Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ + Context: istio_networkingv1alpha3.EnvoyFilter_GATEWAY, + ObjectTypes: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Listener{ + Listener: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch{ + FilterChain: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch_FilterChainMatch{ + Sni: hosts[0], + }, }, }, }, - "patch": principalsToPatch(rbacName, rule.Action, "network", principals), + Patch: principalsToPatch(rbacName, rule.actionProto(), principals), }, nil } -// CreateIngressConfigPatchFromRule creates a network filter patch that can be +// ingressConfigPatchFromRule creates a network filter patch that can be // applied to the `GATEWAY` network filter chain matching the wildcard ingress domain. -func CreateIngressConfigPatchFromRule( +func ingressConfigPatchFromRule( rule *ACLRule, seedIngressDomain, shootID string, alwaysAllowedCIDRs []string, -) map[string]interface{} { +) *istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch { rbacName := "acl-ingress" ingressSuffix := "-" + shootID + "." + seedIngressDomain - return map[string]interface{}{ - "applyTo": "NETWORK_FILTER", - "match": map[string]interface{}{ - "context": "GATEWAY", - "listener": map[string]interface{}{ - "filterChain": map[string]interface{}{ - "sni": "*." + seedIngressDomain, - }, - }, - }, - "patch": map[string]interface{}{ - "operation": "INSERT_FIRST", - "value": map[string]interface{}{ - "name": rbacName, - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", - "rules": map[string]interface{}{ - "action": "ALLOW", - "policies": map[string]interface{}{ - shootID + "-inverse": map[string]interface{}{ - "permissions": []map[string]interface{}{{ - "not_rule": map[string]interface{}{ - "requested_server_name": map[string]interface{}{ - "suffix": ingressSuffix, + rbacFilter := &envoyconfig_rbacv3.RBAC{ + Action: envoyconfig_rbacv3.RBAC_ALLOW, + Policies: map[string]*envoyconfig_rbacv3.Policy{ + shootID + "-inverse": { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_NotRule{ + NotRule: &envoyconfig_rbacv3.Permission{ + Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &matcherv3.StringMatcher{ + MatchPattern: &matcherv3.StringMatcher_Suffix{ + Suffix: ingressSuffix, }, }, - }}, - "principals": []map[string]interface{}{{ - "remote_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": 0, - }, - }}, + }, }, - shootID: map[string]interface{}{ - "permissions": []map[string]interface{}{{ - "requested_server_name": map[string]interface{}{ - "suffix": ingressSuffix, - }, - }}, - "principals": ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), + }, + }, + }, + Principals: []*envoyconfig_rbacv3.Principal{ + { + Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + RemoteIp: &envoy_corev3.CidrRange{ + AddressPrefix: "0.0.0.0", + PrefixLen: wrapperspb.UInt32(0), }, }, }, - "stat_prefix": "envoyrbac", }, }, + shootID: { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &matcherv3.StringMatcher{ + MatchPattern: &matcherv3.StringMatcher_Suffix{ + Suffix: ingressSuffix, + }, + }, + }, + }, + }, + Principals: ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), + }, + }, + } + typedConfig, err := protoMessageToTypedConfig(rbacFilter) + if err != nil { + // this is a error in the code itself, don't return to caller + panic(err) + } + filter := &FilterPatch{ + Name: rbacName, + TypedConfig: typedConfig, + } + return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, + Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ + Context: istio_networkingv1alpha3.EnvoyFilter_GATEWAY, + ObjectTypes: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Listener{ + Listener: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch{ + FilterChain: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch_FilterChainMatch{ + Sni: fmt.Sprintf("*.%s", seedIngressDomain), + }, + }, + }, + }, + Patch: &istio_networkingv1alpha3.EnvoyFilter_Patch{ + Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, + Value: filter.asStructPB(), }, } } -// CreateVPNConfigPatchFromRule creates an HTTP filter patch that can be applied to the +// vpnConfigPatchFromRule creates an HTTP filter patch that can be applied to the // `GATEWAY` HTTP filter chain for the VPN. -func CreateVPNConfigPatchFromRule(rule *ACLRule, +func vpnConfigPatchFromRule(rule *ACLRule, shortShootID, technicalShootID string, alwaysAllowedCIDRs []string, -) (map[string]interface{}, error) { +) *istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch { rbacName := "acl-vpn" - headerMatcher := map[string]interface{}{ - "name": "reversed-vpn", - "string_match": map[string]interface{}{ - // The actual header value will look something like - // `outbound|1194||vpn-seed-server..svc.cluster.local`. - // Include dots in the contains matcher as anchors, to always match the entire technical shoot ID. - // Otherwise, if there was one cluster named `foo` and one named `foo-bar` (in the same project), - // `foo` would effectively inherit the ACL of `foo-bar`. - // We don't match with the full header value to allow service names and ports to change while still making sure - // we catch all traffic targeting this shoot. - "contains": "." + technicalShootID + ".", - }, - } - return map[string]interface{}{ - "applyTo": "HTTP_FILTER", - "match": map[string]interface{}{ - "context": "GATEWAY", - "listener": map[string]interface{}{ - "name": "0.0.0.0_8132", + headerMatcher := envoy_routev3.HeaderMatcher{ + Name: "reversed-vpn", + HeaderMatchSpecifier: &envoy_routev3.HeaderMatcher_StringMatch{ + StringMatch: &matcherv3.StringMatcher{ + MatchPattern: &matcherv3.StringMatcher_Contains{ + // The actual header value will look something like + // `outbound|1194||vpn-seed-server..svc.cluster.local`. + // Include dots in the contains matcher as anchors, to always match the entire technical shoot ID. + // Otherwise, if there was one cluster named `foo` and one named `foo-bar` (in the same project), + // `foo` would effectively inherit the ACL of `foo-bar`. + // We don't match with the full header value to allow service names and ports to change while still making sure + // we catch all traffic targeting this shoot. + Contains: "." + technicalShootID + ".", + }, }, }, - "patch": map[string]interface{}{ - "operation": "INSERT_FIRST", - "value": map[string]interface{}{ - "name": rbacName, - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC", - "rules": map[string]interface{}{ - "action": "ALLOW", - "policies": map[string]interface{}{ - shortShootID + "-inverse": map[string]interface{}{ - "permissions": []map[string]interface{}{{ - "not_rule": map[string]interface{}{ - "header": headerMatcher, - }, - }}, - "principals": []map[string]interface{}{{ - "remote_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": 0, + } + + rbacFilter := &envoyhttp_rbacv3.RBAC{ + RulesStatPrefix: "envoyrbac", + Rules: &envoyconfig_rbacv3.RBAC{ + Action: envoyconfig_rbacv3.RBAC_ALLOW, + Policies: map[string]*envoyconfig_rbacv3.Policy{ + shortShootID + "-inverse": { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_NotRule{ + NotRule: &envoyconfig_rbacv3.Permission{ + Rule: &envoyconfig_rbacv3.Permission_Header{ + Header: &headerMatcher, }, - }}, + }, }, - shortShootID: map[string]interface{}{ - "permissions": []map[string]interface{}{{ - "header": headerMatcher, - }}, - "principals": ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), + }, + }, + Principals: []*envoyconfig_rbacv3.Principal{ + { + Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + RemoteIp: &envoy_corev3.CidrRange{ + AddressPrefix: "0.0.0.0", + PrefixLen: wrapperspb.UInt32(0), + }, }, }, }, - "stat_prefix": "envoyrbac", + }, + shortShootID: { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_Header{ + Header: &headerMatcher, + }, + }, + }, + Principals: ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), }, }, }, - }, nil + } + typedConfig, err := protoMessageToTypedConfig(rbacFilter) + if err != nil { + // this is a error in the code itself, don't return to caller + panic(err) + } + filterPatch := &FilterPatch{ + Name: rbacName, + TypedConfig: typedConfig, + } + return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + ApplyTo: istio_networkingv1alpha3.EnvoyFilter_HTTP_FILTER, + Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ + Context: istio_networkingv1alpha3.EnvoyFilter_GATEWAY, + ObjectTypes: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Listener{ + Listener: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch{ + Name: "0.0.0.0_8132", + }, + }, + }, + Patch: &istio_networkingv1alpha3.EnvoyFilter_Patch{ + Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, + Value: filterPatch.asStructPB(), + }, + } } // CreateInternalFilterPatchFromRule combines an ACLRule, the @@ -246,34 +335,50 @@ func CreateInternalFilterPatchFromRule( rule *ACLRule, alwaysAllowedCIDRs []string, shootSpecificCIDRs []string, -) (map[string]interface{}, error) { +) *FilterPatch { rbacName := "acl-internal" principals := ruleCIDRsToPrincipal(rule, append(alwaysAllowedCIDRs, shootSpecificCIDRs...)) + rbacFilter := newRBACFilter(rbacName, rule.actionProto(), principals) + typedConfig, err := protoMessageToTypedConfig(rbacFilter) + if err != nil { + // this is a error in the code itself, don't return to caller + panic(err) + } - return map[string]interface{}{ - "name": rbacName + "-" + strings.ToLower(rule.Type), - "typed_config": typedConfigToPatch(rbacName, rule.Action, "network", principals), - }, nil + return &FilterPatch{ + Name: rbacName + "-" + strings.ToLower(rule.Type), + TypedConfig: typedConfig, + } } // ruleCIDRsToPrincipal translates a list of strings in the form "0.0.0.0/0" // into a list of envoy principals. The function checks for the rule action: If // the action is "ALLOW", the alwaysAllowedCIDRs are appended to the principals // to guarantee the downstream flow for these CIDRs is not blocked. -func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []map[string]interface{} { - principals := []map[string]interface{}{} +func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoyconfig_rbacv3.Principal { + principals := []*envoyconfig_rbacv3.Principal{} for _, cidr := range rule.Cidrs { prefix, length, err := getPrefixAndPrefixLength(cidr) if err != nil { continue } - principals = append(principals, map[string]interface{}{ - strings.ToLower(rule.Type): map[string]interface{}{ - "address_prefix": prefix, - "prefix_len": length, - }, - }) + cidrRange := envoy_corev3.CidrRange{ + AddressPrefix: prefix, + PrefixLen: wrapperspb.UInt32(uint32(length)), + } + p := new(envoyconfig_rbacv3.Principal) + switch rule.Type { + case "source_ip": + p.Identifier = &envoyconfig_rbacv3.Principal_SourceIp{SourceIp: &cidrRange} + case "remote_ip": + p.Identifier = &envoyconfig_rbacv3.Principal_RemoteIp{RemoteIp: &cidrRange} + case "direct_remote_ip": + p.Identifier = &envoyconfig_rbacv3.Principal_DirectRemoteIp{DirectRemoteIp: &cidrRange} + default: + continue + } + principals = append(principals, p) } // if the rule has action "ALLOW" (which means "limit the access to only the @@ -285,10 +390,12 @@ func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []map[stri if err != nil { continue } - principals = append(principals, map[string]interface{}{ - "remote_ip": map[string]interface{}{ - "address_prefix": prefix, - "prefix_len": length, + principals = append(principals, &envoyconfig_rbacv3.Principal{ + Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + RemoteIp: &envoy_corev3.CidrRange{ + AddressPrefix: prefix, + PrefixLen: wrapperspb.UInt32(uint32(length)), + }, }, }) } @@ -310,31 +417,55 @@ func getPrefixAndPrefixLength(cidr string) (prefix string, prefixLen int, err er } func principalsToPatch( - rbacName, ruleAction, filterType string, principals []map[string]interface{}, -) map[string]interface{} { - return map[string]interface{}{ - "operation": "INSERT_FIRST", - "value": map[string]interface{}{ - "name": rbacName, - "typed_config": typedConfigToPatch(rbacName, ruleAction, filterType, principals), - }, + rbacName string, ruleAction envoyconfig_rbacv3.RBAC_Action, principals []*envoyconfig_rbacv3.Principal, +) *istio_networkingv1alpha3.EnvoyFilter_Patch { + rbacFilter := newRBACFilter(rbacName, ruleAction, principals) + typedConfig, err := protoMessageToTypedConfig(rbacFilter) + if err != nil { + // this is a error in the code itself, don't return to caller + panic(err) + } + filter := &FilterPatch{ + Name: rbacName, + TypedConfig: typedConfig, + } + return &istio_networkingv1alpha3.EnvoyFilter_Patch{ + Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, + Value: filter.asStructPB(), } } -func typedConfigToPatch(rbacName, ruleAction, filterType string, principals []map[string]interface{}) map[string]interface{} { - return map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters." + filterType + ".rbac.v3.RBAC", - "stat_prefix": "envoyrbac", - "rules": map[string]interface{}{ - "action": strings.ToUpper(ruleAction), - "policies": map[string]interface{}{ - rbacName: map[string]interface{}{ - "permissions": []map[string]interface{}{ - {"any": true}, +func newRBACFilter(rbacName string, ruleAction envoyconfig_rbacv3.RBAC_Action, principals []*envoyconfig_rbacv3.Principal) *envoynetwork_rbacv3.RBAC { + return &envoynetwork_rbacv3.RBAC{ + StatPrefix: "envoyrbac", + Rules: &envoyconfig_rbacv3.RBAC{ + Action: ruleAction, + Policies: map[string]*envoyconfig_rbacv3.Policy{ + rbacName: { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_Any{ + Any: true, + }, + }, }, - "principals": principals, + Principals: principals, }, }, }, } } + +func protoMessageToTypedConfig(m proto.Message) (*structpb.Struct, error) { + raw, err := protojson.Marshal(m) + if err != nil { + return nil, err + } + s := new(structpb.Struct) + if err := protojson.Unmarshal(raw, s); err != nil { + return nil, err + } + typeName := fmt.Sprintf("type.googleapis.com/%s", proto.MessageName(m)) + s.Fields["@type"] = structpb.NewStringValue(typeName) + return s, nil +} diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go new file mode 100644 index 000000000..c0df8ecfa --- /dev/null +++ b/pkg/webhook/webhook.go @@ -0,0 +1,172 @@ +package webhook + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + v1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "google.golang.org/protobuf/types/known/structpb" + + "gomodules.xyz/jsonpatch/v2" + istionetworkingClientGo "istio.io/client-go/pkg/apis/networking/v1alpha3" + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/gardener-extension-acl/pkg/controller" + "github.com/stackitcloud/gardener-extension-acl/pkg/envoyfilters" + "github.com/stackitcloud/gardener-extension-acl/pkg/extensionspec" + "github.com/stackitcloud/gardener-extension-acl/pkg/helper" +) + +const ( + // ExtensionName contains the ACl extension name. + ExtensionName = "acl" +) + +// EnvoyFilterWebhook is a service struct that defines functions to handle +// admission requests for EnvoyFilters. +type EnvoyFilterWebhook struct { + Client client.Client + Decoder admission.Decoder + AdditionalAllowedCIDRs []string +} + +// Handle receives incoming admission requests for EnvoyFilters and returns a +// response. The response contains patches for specific EnvoyFilters, which need +// to be patched for the ACL extension to be able to control all filter chains. +// +//nolint:gocritic // the signature is forced by kubebuilder +func (e *EnvoyFilterWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { + filter := &istionetworkingClientGo.EnvoyFilter{} + if err := e.Decoder.Decode(req, filter); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + return e.createAdmissionResponse(ctx, filter) +} + +func (e *EnvoyFilterWebhook) createAdmissionResponse( + ctx context.Context, + filter *istionetworkingClientGo.EnvoyFilter, +) admission.Response { + // filter out envoyfilters that are not managed by this webhook + if !strings.HasPrefix(filter.Name, v1beta1constants.TechnicalIDPrefix) { + return admission.Allowed("requested object is not an EnvoyFilter managed by this webhook") + } + + aclExtension := &extensionsv1alpha1.Extension{} + err := e.Client.Get(ctx, types.NamespacedName{Name: ExtensionName, Namespace: filter.Name}, aclExtension) + + if client.IgnoreNotFound(err) != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + // if an error occured or the extension is in deletion, just allow without + // introducing any patches + if err != nil || !aclExtension.DeletionTimestamp.IsZero() { + return admission.Allowed(fmt.Sprintf("extension %s not enabled for shoot %s or is in deletion", ExtensionName, filter.Name)) + } + + extSpec := &extensionspec.ExtensionSpec{} + if err := json.Unmarshal(aclExtension.Spec.ProviderConfig.Raw, extSpec); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + if err := controller.ValidateExtensionSpec(extSpec); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + cluster, err := helper.GetClusterForExtension(ctx, e.Client, aclExtension) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + var alwaysAllowedCIDRs []string + var shootSpecificCIRDs []string + + alwaysAllowedCIDRs = append(alwaysAllowedCIDRs, helper.GetSeedSpecificAllowedCIDRs(cluster.Seed)...) + + if len(e.AdditionalAllowedCIDRs) >= 1 { + alwaysAllowedCIDRs = append(alwaysAllowedCIDRs, e.AdditionalAllowedCIDRs...) + } + + // Gardener supports workerless Shoots. These don't have an associated + // Infrastructure object and don't need Node- or Pod-specific CIDRs to be + // allowed. Therefore, skip these steps for workerless Shoots. + if !v1beta1helper.IsWorkerless(cluster.Shoot) { + shootSpecificCIRDs = append(shootSpecificCIRDs, helper.GetShootNodeSpecificAllowedCIDRs(cluster.Shoot)...) + shootSpecificCIRDs = append(shootSpecificCIRDs, helper.GetShootPodSpecificAllowedCIDRs(cluster.Shoot)...) + + infra, err := helper.GetInfrastructureForExtension(ctx, e.Client, aclExtension, cluster.Shoot.Name) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + providerSpecificCIRDs, err := helper.GetProviderSpecificAllowedCIDRs(infra) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + shootSpecificCIRDs = append(shootSpecificCIRDs, providerSpecificCIRDs...) + } + + var originalFilter *structpb.Struct + for _, configpatch := range filter.Spec.ConfigPatches { + patch := configpatch.Patch.Value + filters, ok := patch.Fields["filters"] + if !ok { + continue + } + filterList := filters.GetListValue() + if filterList == nil { + continue + } + for _, v := range filterList.GetValues() { + structVal := v.GetStructValue() + if structVal == nil { + continue + } + name, ok := structVal.Fields["name"] + if !ok { + continue + } + if name.GetStringValue() == "envoy.filters.network.tcp_proxy" { + originalFilter = structVal + break + } + } + } + if originalFilter == nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("filter with name \"envoy.filters.network.tcp_proxy\" not present in EnvoyFilter %s/%s", filter.Namespace, filter.Name)) + } + filterPatch := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) + + // make sure the original filter is the last + filterPatches := []map[string]interface{}{filterPatch.AsMap(), originalFilter.AsMap()} + + return buildAdmissionResponseWithFilterPatches(filterPatches) +} + +func buildAdmissionResponseWithFilterPatches(filters []map[string]interface{}) admission.Response { + return admission.Response{ + Patches: []jsonpatch.Operation{ + { + Operation: "replace", + Path: "/spec/configPatches/0/patch/value/filters", + Value: filters, + }, + }, + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + PatchType: ptr.To(admissionv1.PatchTypeJSONPatch), + }, + } +} From 988aa4a10be47088a5d4b8e8b0a65cbe5faa96e9 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 15:05:49 +0100 Subject: [PATCH 02/14] test: adjust golden files Signed-off-by: Lukas Hoehl --- .../testdata/apiEnvoyFilterSpecWithOneAllowRule.yaml | 3 +-- .../testdata/ingressEnvoyFilterSpecWithOneAllowRule.yaml | 1 - .../testdata/legacyVPNEnvoyFilterSpecWithOneAllowRule.yaml | 1 - pkg/envoyfilters/testdata/singleFiltersAllowEntry.yaml | 1 - .../testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml | 3 +-- 5 files changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/envoyfilters/testdata/apiEnvoyFilterSpecWithOneAllowRule.yaml b/pkg/envoyfilters/testdata/apiEnvoyFilterSpecWithOneAllowRule.yaml index 85ffc7681..f62e31e8c 100644 --- a/pkg/envoyfilters/testdata/apiEnvoyFilterSpecWithOneAllowRule.yaml +++ b/pkg/envoyfilters/testdata/apiEnvoyFilterSpecWithOneAllowRule.yaml @@ -12,7 +12,6 @@ configPatches: typed_config: '@type': type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC rules: - action: ALLOW policies: acl-api: permissions: @@ -31,4 +30,4 @@ configPatches: workloadSelector: labels: app: istio-ingressgateway - istio: ingressgateway \ No newline at end of file + istio: ingressgateway diff --git a/pkg/envoyfilters/testdata/ingressEnvoyFilterSpecWithOneAllowRule.yaml b/pkg/envoyfilters/testdata/ingressEnvoyFilterSpecWithOneAllowRule.yaml index 0a0f9442c..e5a8f14b6 100644 --- a/pkg/envoyfilters/testdata/ingressEnvoyFilterSpecWithOneAllowRule.yaml +++ b/pkg/envoyfilters/testdata/ingressEnvoyFilterSpecWithOneAllowRule.yaml @@ -12,7 +12,6 @@ configPatches: typed_config: '@type': type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC rules: - action: ALLOW policies: bar--foo: permissions: diff --git a/pkg/envoyfilters/testdata/legacyVPNEnvoyFilterSpecWithOneAllowRule.yaml b/pkg/envoyfilters/testdata/legacyVPNEnvoyFilterSpecWithOneAllowRule.yaml index 760815129..890903ed4 100644 --- a/pkg/envoyfilters/testdata/legacyVPNEnvoyFilterSpecWithOneAllowRule.yaml +++ b/pkg/envoyfilters/testdata/legacyVPNEnvoyFilterSpecWithOneAllowRule.yaml @@ -11,7 +11,6 @@ configPatches: typed_config: '@type': type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC rules: - action: ALLOW policies: acl-vpn-inverse: permissions: diff --git a/pkg/envoyfilters/testdata/singleFiltersAllowEntry.yaml b/pkg/envoyfilters/testdata/singleFiltersAllowEntry.yaml index feb69582e..b451bb8eb 100644 --- a/pkg/envoyfilters/testdata/singleFiltersAllowEntry.yaml +++ b/pkg/envoyfilters/testdata/singleFiltersAllowEntry.yaml @@ -2,7 +2,6 @@ name: acl-internal-remote_ip typed_config: '@type': type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC rules: - action: ALLOW policies: acl-internal: permissions: diff --git a/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml b/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml index 8a0f4db9b..86b24bb44 100644 --- a/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml +++ b/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml @@ -11,7 +11,6 @@ configPatches: typed_config: '@type': type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC rules: - action: ALLOW policies: bar--foo-inverse: permissions: @@ -40,7 +39,7 @@ configPatches: - remote_ip: address_prefix: 10.96.0.0 prefix_len: 11 - stat_prefix: envoyrbac + rules_stat_prefix: envoyrbac workloadSelector: labels: app: istio-ingressgateway From 7ce9edfa40edd0336df4d5f94932ae830bd0c7b0 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 15:05:57 +0100 Subject: [PATCH 03/14] test: adapt to types Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters.go | 63 +-- pkg/envoyfilters/envoyfilters_test.go | 45 +- pkg/webhook/webhook_test.go | 575 ++++++++++++++++++++++++++ 3 files changed, 633 insertions(+), 50 deletions(-) create mode 100644 pkg/webhook/webhook_test.go diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 33bbf5491..5ebeb7391 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -164,49 +164,52 @@ func ingressConfigPatchFromRule( rbacName := "acl-ingress" ingressSuffix := "-" + shootID + "." + seedIngressDomain - rbacFilter := &envoyconfig_rbacv3.RBAC{ - Action: envoyconfig_rbacv3.RBAC_ALLOW, - Policies: map[string]*envoyconfig_rbacv3.Policy{ - shootID + "-inverse": { - Permissions: []*envoyconfig_rbacv3.Permission{ - { - Rule: &envoyconfig_rbacv3.Permission_NotRule{ - NotRule: &envoyconfig_rbacv3.Permission{ - Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ - RequestedServerName: &matcherv3.StringMatcher{ - MatchPattern: &matcherv3.StringMatcher_Suffix{ - Suffix: ingressSuffix, + rbacFilter := &envoynetwork_rbacv3.RBAC{ + StatPrefix: "envoyrbac", + Rules: &envoyconfig_rbacv3.RBAC{ + Action: envoyconfig_rbacv3.RBAC_ALLOW, + Policies: map[string]*envoyconfig_rbacv3.Policy{ + shootID + "-inverse": { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_NotRule{ + NotRule: &envoyconfig_rbacv3.Permission{ + Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &matcherv3.StringMatcher{ + MatchPattern: &matcherv3.StringMatcher_Suffix{ + Suffix: ingressSuffix, + }, }, }, }, }, }, }, - }, - Principals: []*envoyconfig_rbacv3.Principal{ - { - Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ - RemoteIp: &envoy_corev3.CidrRange{ - AddressPrefix: "0.0.0.0", - PrefixLen: wrapperspb.UInt32(0), + Principals: []*envoyconfig_rbacv3.Principal{ + { + Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + RemoteIp: &envoy_corev3.CidrRange{ + AddressPrefix: "0.0.0.0", + PrefixLen: wrapperspb.UInt32(0), + }, }, }, }, }, - }, - shootID: { - Permissions: []*envoyconfig_rbacv3.Permission{ - { - Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ - RequestedServerName: &matcherv3.StringMatcher{ - MatchPattern: &matcherv3.StringMatcher_Suffix{ - Suffix: ingressSuffix, + shootID: { + Permissions: []*envoyconfig_rbacv3.Permission{ + { + Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &matcherv3.StringMatcher{ + MatchPattern: &matcherv3.StringMatcher_Suffix{ + Suffix: ingressSuffix, + }, }, }, }, }, + Principals: ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), }, - Principals: ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs), }, }, } @@ -457,7 +460,9 @@ func newRBACFilter(rbacName string, ruleAction envoyconfig_rbacv3.RBAC_Action, p } func protoMessageToTypedConfig(m proto.Message) (*structpb.Struct, error) { - raw, err := protojson.Marshal(m) + raw, err := protojson.MarshalOptions{ + UseProtoNames: true, + }.Marshal(m) if err != nil { return nil, err } diff --git a/pkg/envoyfilters/envoyfilters_test.go b/pkg/envoyfilters/envoyfilters_test.go index a4cec48ac..67d0df8af 100644 --- a/pkg/envoyfilters/envoyfilters_test.go +++ b/pkg/envoyfilters/envoyfilters_test.go @@ -1,6 +1,7 @@ package envoyfilters import ( + "encoding/json" "os" "path" @@ -8,8 +9,10 @@ import ( "github.com/gardener/gardener/pkg/extensions" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "gopkg.in/yaml.v3" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" ) var _ = Describe("EnvoyFilter Unit Tests", func() { @@ -52,7 +55,7 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { result, err := BuildAPIEnvoyFilterSpecForHelmChart(rule, hosts, alwaysAllowedCIDRs, labels) Expect(err).ToNot(HaveOccurred()) - checkIfMapEqualsYAML(result, "apiEnvoyFilterSpecWithOneAllowRule.yaml") + checkIfFilterEquals(result, "apiEnvoyFilterSpecWithOneAllowRule.yaml") }) }) }) @@ -67,7 +70,7 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { } ingressEnvoyFilterSpec := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) - checkIfMapEqualsYAML(ingressEnvoyFilterSpec, "ingressEnvoyFilterSpecWithOneAllowRule.yaml") + checkIfFilterEquals(ingressEnvoyFilterSpec, "ingressEnvoyFilterSpecWithOneAllowRule.yaml") }) It("Should not create an envoyFilter spec when seed has no ingress", func() { rule := createRule("ALLOW", "remote_ip", "10.180.0.0/16") @@ -77,7 +80,7 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { "istio": "ingressgateway", } ingressEnvoyFilterSpec := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) - Expect(ingressEnvoyFilterSpec["ingressEnvoyFilterSpec"]).To(BeNil()) + Expect(ingressEnvoyFilterSpec).To(BeNil()) }) }) }) @@ -90,10 +93,8 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { "app": "istio-ingressgateway", "istio": "ingressgateway", } - result, err := BuildVPNEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) - - Expect(err).ToNot(HaveOccurred()) - checkIfMapEqualsYAML(result, "vpnEnvoyFilterSpecWithOneAllowRule.yaml") + result := BuildVPNEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + checkIfFilterEquals(result, "vpnEnvoyFilterSpecWithOneAllowRule.yaml") }) }) }) @@ -103,10 +104,8 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { It("Should create a filter spec matching the expected one, including the always allowed CIDRs", func() { rule := createRule("ALLOW", "remote_ip", "0.0.0.0/0") - result, err := CreateInternalFilterPatchFromRule(rule, alwaysAllowedCIDRs, []string{}) - - Expect(err).ToNot(HaveOccurred()) - checkIfMapEqualsYAML(result, "singleFiltersAllowEntry.yaml") + result := CreateInternalFilterPatchFromRule(rule, alwaysAllowedCIDRs, []string{}) + checkIfFilterEquals(result, "singleFiltersAllowEntry.yaml") }) }) }) @@ -123,7 +122,6 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { }) }) }) - }) //nolint:unparam // action currently only accepts ALLOW but that might change, so we leave the parameterization @@ -137,19 +135,24 @@ func createRule(action, ruleType, cidr string) *ACLRule { } } -// checkIfMapEqualsYAML takes a map as input, and tries to compare its +// checkIfFilterEqualsYAML takes a map as input, and tries to compare its // marshaled contents to the string coming from the specified testdata file. // Fails the test if strings differ. The file contents are unmarshaled and // marshaled again to guarantee the strings are comparable. -func checkIfMapEqualsYAML(input map[string]interface{}, relTestingFilePath string) { - goldenYAMLByteArray, err := os.ReadFile(path.Join("./testdata", relTestingFilePath)) +func checkIfFilterEquals(input any, relTestingFilePath string) { + goldenYAMLBytes, err := os.ReadFile(path.Join("./testdata", relTestingFilePath)) Expect(err).ToNot(HaveOccurred()) - goldenMap := map[string]interface{}{} - Expect(yaml.Unmarshal(goldenYAMLByteArray, goldenMap)).To(Succeed()) - goldenYAMLProcessedByteArray, err := yaml.Marshal(goldenMap) + + goldenJSON, err := yaml.YAMLToJSON(goldenYAMLBytes) Expect(err).ToNot(HaveOccurred()) - inputByteArray, err := yaml.Marshal(input) + var actual []byte + if m, ok := input.(proto.Message); ok { + actual, err = protojson.Marshal(m) + } else { + actual, err = json.Marshal(input) + } Expect(err).ToNot(HaveOccurred()) - Expect(string(inputByteArray)).To(Equal(string(goldenYAMLProcessedByteArray))) + + Expect(actual).To(MatchJSON(goldenJSON)) } diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go new file mode 100644 index 000000000..852934631 --- /dev/null +++ b/pkg/webhook/webhook_test.go @@ -0,0 +1,575 @@ +package webhook + +import ( + "context" + "encoding/json" + "os" + "path" + "strings" + + openstackv1alpha1 "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1" + "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack" + gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + istionetworkingClientGo "istio.io/client-go/pkg/apis/networking/v1alpha3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/stackitcloud/gardener-extension-acl/pkg/envoyfilters" + "github.com/stackitcloud/gardener-extension-acl/pkg/extensionspec" +) + +var _ = Describe("webhook unit test", func() { + var ( + e *EnvoyFilterWebhook + ext *extensionsv1alpha1.Extension + cluster *extensionsv1alpha1.Cluster + infra *extensionsv1alpha1.Infrastructure + namespace string + name string + ) + + BeforeEach(func() { + name = "some-shoot" + namespace = createNewNamespace() + infra = getNewInfrastructure(namespace, name, "non-existent", []byte("{}"), []byte("{}")) + Expect(k8sClient.Create(ctx, infra)).To(Succeed()) + + // set up default shoot part of cluster resource + shoot := &gardencorev1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gardencorev1beta1.ShootSpec{ + Networking: &gardencorev1beta1.Networking{ + Nodes: ptr.To("10.250.0.0/16"), + Pods: ptr.To("100.96.0.0/11"), + Services: ptr.To("100.64.0.0/13"), + Type: ptr.To("calico"), + }, + // we need a provider section with at least one worker pool, + // otherwise the Shoot will be considered workerless + Provider: gardencorev1beta1.Provider{ + Workers: []gardencorev1beta1.Worker{ + { + Name: "workerpool", + }, + }, + }, + }, + } + + // set up default seed part of cluster resource + seed := &gardencorev1beta1.Seed{ + Spec: gardencorev1beta1.SeedSpec{ + Networks: gardencorev1beta1.SeedNetworks{ + Nodes: ptr.To("100.250.0.0/16"), + Pods: "10.96.0.0/11", + Services: "10.64.0.0/13", + }, + }, + } + + cluster = getNewCluster(namespace, shoot, seed) + Expect(k8sClient.Create(ctx, cluster)).To(Succeed()) + + e = getNewWebhook() + }) + + AfterEach(func() { + deleteNamespace(namespace) + }) + + Describe("createAdmissionResponse", func() { + When("the name of the EnvoyFilter doesn't start with 'shoot-'", func() { + It("issues no patch for the EnvoyFilter", func() { + df := getEnvoyFilterFromFile("non-shoot-envoyfilter") + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + Expect(ar.Result.Message).To(ContainSubstring("not an EnvoyFilter managed by this webhook")) + Expect(ar.Patch).To(BeEmpty()) + }) + }) + + When("there is no extension", func() { + When("the shoot uses the legacy technical ID format 'shoot-'", func() { + It("issues no patch for the EnvoyFilter", func() { + df := getEnvoyFilterFromFile("shoot-foo-bar") + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + Expect(ar.Result.Message).To(ContainSubstring("not enabled for shoot")) + Expect(ar.Patch).To(BeEmpty()) + }) + }) + + When("the shoot uses the current technical ID format 'shoot--'", func() { + It("issues no patch for the EnvoyFilter", func() { + df := getEnvoyFilterFromFile(namespace) + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + Expect(ar.Result.Message).To(ContainSubstring("not enabled for shoot")) + Expect(ar.Patch).To(BeEmpty()) + }) + }) + }) + + When("there is an extension resource with one DENY rule", func() { + extSpec := getExtensionSpec() + + BeforeEach(func() { + addRuleToSpec(extSpec, "DENY", "source_ip", "0.0.0.0/0") + ext = getNewExtension(namespace, *extSpec) + + Expect(k8sClient.Create(ctx, ext)).To(Succeed()) + }) + + AfterEach(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) + }) + + It("patches this rule into the filters object", func() { + df := getEnvoyFilterFromFile(namespace) + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + + expectedFilters := []map[string]interface{}{ + { + "name": "acl-internal-source_ip", + "typed_config": map[string]interface{}{ + "stat_prefix": "envoyrbac", + "rules": map[string]interface{}{ + "policies": map[string]interface{}{ + "acl-internal": map[string]interface{}{ + "permissions": []map[string]interface{}{ + { + "any": true, + }, + }, + "principals": []map[string]interface{}{ + { + "source_ip": map[string]interface{}{ + "address_prefix": "0.0.0.0", + "prefix_len": 0, + }, + }, + }, + }, + }, + "action": "DENY", + }, + "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", + }, + }, + + { + "name": "envoy.filters.network.tcp_proxy", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", + "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + }, + }, + } + + Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) + }) + }) + + When("there is an extension resource with one ALLOW rule", func() { + extSpec := getExtensionSpec() + + BeforeEach(func() { + addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") + ext = getNewExtension(namespace, *extSpec) + + Expect(k8sClient.Create(ctx, ext)).To(Succeed()) + }) + + AfterEach(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) + }) + + It("patches this rule into the filters object, including CIDRs for Seed|Shoot nodes and pods", func() { + df := getEnvoyFilterFromFile(namespace) + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + + expectedFilters := []map[string]interface{}{ + { + "name": "acl-internal-source_ip", + "typed_config": map[string]interface{}{ + "stat_prefix": "envoyrbac", + "rules": map[string]interface{}{ + "policies": map[string]interface{}{ + "acl-internal": map[string]interface{}{ + "permissions": []map[string]interface{}{ + { + "any": true, + }, + }, + "principals": []map[string]interface{}{ + { + "source_ip": map[string]interface{}{ + "address_prefix": "0.0.0.0", + "prefix_len": 0, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "100.250.0.0", + "prefix_len": 16, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.96.0.0", + "prefix_len": 11, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.250.0.0", + "prefix_len": 16, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "100.96.0.0", + "prefix_len": 11, + }, + }, + }, + }, + }, + "action": "ALLOW", + }, + "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", + }, + }, + + { + "name": "envoy.filters.network.tcp_proxy", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", + "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + }, + }, + } + + Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) + }) + }) + + When("there is an extension resource with one ALLOW rule and infra is of type OpenStack", func() { + extSpec := getExtensionSpec() + + BeforeEach(func() { + addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") + ext = getNewExtension(namespace, *extSpec) + + Expect(k8sClient.Create(ctx, ext)).To(Succeed()) + + infra.Spec.Type = openstack.Type + Expect(k8sClient.Update(ctx, infra)).To(Succeed()) + + infraStatusJSON, err := json.Marshal(&openstackv1alpha1.InfrastructureStatus{ + TypeMeta: metav1.TypeMeta{ + Kind: "InfrastructureStatus", + APIVersion: "openstack.provider.extensions.gardener.cloud/v1alpha1", + }, + Networks: openstackv1alpha1.NetworkStatus{ + Router: openstackv1alpha1.RouterStatus{ + ID: "router-id", + IP: "10.9.8.7", + }, + }, + }) + Expect(err).To(BeNil()) + + infra.Status = extensionsv1alpha1.InfrastructureStatus{ + DefaultStatus: extensionsv1alpha1.DefaultStatus{ + ProviderStatus: &runtime.RawExtension{ + Raw: infraStatusJSON, + }, + }, + } + Expect(k8sClient.Status().Update(ctx, infra)).To(Succeed()) + }) + + AfterEach(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) + }) + + It("patches this rule into the filters object, including CIDRs for Seed|Shoot nodes and pods and also the OpenStack router IP", func() { + df := getEnvoyFilterFromFile(namespace) + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + + expectedFilters := []map[string]interface{}{ + { + "name": "acl-internal-source_ip", + "typed_config": map[string]interface{}{ + "stat_prefix": "envoyrbac", + "rules": map[string]interface{}{ + "policies": map[string]interface{}{ + "acl-internal": map[string]interface{}{ + "permissions": []map[string]interface{}{ + { + "any": true, + }, + }, + "principals": []map[string]interface{}{ + { + "source_ip": map[string]interface{}{ + "address_prefix": "0.0.0.0", + "prefix_len": 0, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "100.250.0.0", + "prefix_len": 16, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.96.0.0", + "prefix_len": 11, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.250.0.0", + "prefix_len": 16, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "100.96.0.0", + "prefix_len": 11, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.9.8.7", + "prefix_len": 32, + }, + }, + }, + }, + }, + "action": "ALLOW", + }, + "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", + }, + }, + + { + "name": "envoy.filters.network.tcp_proxy", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", + "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + }, + }, + } + + Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) + }) + }) + + When("the Shoot is workerless, and there is one allow rule", func() { + extSpec := getExtensionSpec() + + BeforeEach(func() { + addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") + ext = getNewExtension(namespace, *extSpec) + Expect(k8sClient.Create(ctx, ext)).To(Succeed()) + + // define a workerless shoot + shoot := &gardencorev1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: gardencorev1beta1.ShootSpec{}, + } + shootJSON, err := json.Marshal(shoot) + Expect(err).To(BeNil()) + + cluster.Spec.Shoot = runtime.RawExtension{Raw: shootJSON} + Expect(k8sClient.Update(ctx, cluster)).To(Succeed()) + }) + + AfterEach(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) + }) + + It("patches only the rule into the filters object, and no CIDRs for Seed|Shoot nodes and pods", func() { + df := getEnvoyFilterFromFile(namespace) + + ar := e.createAdmissionResponse(context.Background(), df) + + Expect(ar.Allowed).To(BeTrue()) + + expectedFilters := []map[string]interface{}{ + { + "name": "acl-internal-source_ip", + "typed_config": map[string]interface{}{ + "stat_prefix": "envoyrbac", + "rules": map[string]interface{}{ + "policies": map[string]interface{}{ + "acl-internal": map[string]interface{}{ + "permissions": []map[string]interface{}{ + { + "any": true, + }, + }, + "principals": []map[string]interface{}{ + { + "source_ip": map[string]interface{}{ + "address_prefix": "0.0.0.0", + "prefix_len": 0, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "100.250.0.0", + "prefix_len": 16, + }, + }, + { + "remote_ip": map[string]interface{}{ + "address_prefix": "10.96.0.0", + "prefix_len": 11, + }, + }, + }, + }, + }, + "action": "ALLOW", + }, + "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", + }, + }, + + { + "name": "envoy.filters.network.tcp_proxy", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", + "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", + }, + }, + } + + Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) + }) + }) + }) +}) + +func getNewWebhook() *EnvoyFilterWebhook { + decoder := admission.NewDecoder(clientScheme) + return &EnvoyFilterWebhook{ + Client: k8sClient, + Decoder: decoder, + } +} + +func getNewCluster(namespace string, shoot *gardencorev1beta1.Shoot, seed *gardencorev1beta1.Seed) *extensionsv1alpha1.Cluster { + shootJSON, err := json.Marshal(shoot) + Expect(err).To(BeNil()) + + seedJSON, err := json.Marshal(seed) + Expect(err).To(BeNil()) + + return &extensionsv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + Spec: extensionsv1alpha1.ClusterSpec{ + CloudProfile: runtime.RawExtension{Raw: []byte("{}")}, + Seed: runtime.RawExtension{Raw: seedJSON}, + Shoot: runtime.RawExtension{Raw: shootJSON}, + }, + } +} + +func getNewInfrastructure( + namespace, name, typeName string, + providerConfigJSON, providerStatusJSON []byte, +) *extensionsv1alpha1.Infrastructure { + return &extensionsv1alpha1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Spec: extensionsv1alpha1.InfrastructureSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: typeName, + ProviderConfig: &runtime.RawExtension{ + Raw: providerConfigJSON, + }, + }, + }, + Status: extensionsv1alpha1.InfrastructureStatus{ + DefaultStatus: extensionsv1alpha1.DefaultStatus{ + ProviderStatus: &runtime.RawExtension{ + Raw: providerStatusJSON, + }, + }, + }, + } +} + +func getExtensionSpec() *extensionspec.ExtensionSpec { + return &extensionspec.ExtensionSpec{ + Rule: &envoyfilters.ACLRule{}, + } +} + +func addRuleToSpec(extSpec *extensionspec.ExtensionSpec, action, ruleType, cidr string) { + extSpec.Rule = &envoyfilters.ACLRule{ + Cidrs: []string{ + cidr, + }, + Action: action, + Type: ruleType, + } +} + +// getEnvoyFilterFromFile takes the technical shoot ID as a parameter to render +// into the JSON tempate file. Returns both the JSON representation as string +// and the struct type. +func getEnvoyFilterFromFile(technicalID string) (filter *istionetworkingClientGo.EnvoyFilter) { + filterSpecJSON, err := os.ReadFile(path.Join("./testdata", "defaultEnvoyFilter.json")) + Expect(err).ShouldNot(HaveOccurred()) + + templatedFilterSpec := strings.ReplaceAll(string(filterSpecJSON), "{{TECHNICAL-SHOOT-ID}}", technicalID) + + filter = &istionetworkingClientGo.EnvoyFilter{} + + Expect(json.Unmarshal([]byte(templatedFilterSpec), filter)).To(Succeed()) + + return filter +} From f2ef58805851e1055df821e3feea80d1d9d985a2 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 15:52:03 +0100 Subject: [PATCH 04/14] test: adapt webhook tests Signed-off-by: Lukas Hoehl --- pkg/webhook/webhook.go | 3 +- pkg/webhook/webhook_test.go | 74 ++++++++++++++++++++----------------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index c0df8ecfa..572007cda 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -10,9 +10,8 @@ import ( v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" v1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - "google.golang.org/protobuf/types/known/structpb" - "gomodules.xyz/jsonpatch/v2" + "google.golang.org/protobuf/types/known/structpb" istionetworkingClientGo "istio.io/client-go/pkg/apis/networking/v1alpha3" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/types" diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index 852934631..2c7e223e8 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -154,19 +154,19 @@ var _ = Describe("webhook unit test", func() { "rules": map[string]interface{}{ "policies": map[string]interface{}{ "acl-internal": map[string]interface{}{ - "permissions": []map[string]interface{}{ + "permissions": buildInterfaceSlice([]map[string]interface{}{ { "any": true, }, - }, - "principals": []map[string]interface{}{ + }), + "principals": buildInterfaceSlice([]map[string]interface{}{ { "source_ip": map[string]interface{}{ "address_prefix": "0.0.0.0", - "prefix_len": 0, + "prefix_len": float64(0), }, }, - }, + }), }, }, "action": "DENY", @@ -218,46 +218,45 @@ var _ = Describe("webhook unit test", func() { "rules": map[string]interface{}{ "policies": map[string]interface{}{ "acl-internal": map[string]interface{}{ - "permissions": []map[string]interface{}{ + "permissions": buildInterfaceSlice([]map[string]interface{}{ { "any": true, }, - }, - "principals": []map[string]interface{}{ + }), + "principals": buildInterfaceSlice([]map[string]any{ { "source_ip": map[string]interface{}{ "address_prefix": "0.0.0.0", - "prefix_len": 0, + "prefix_len": float64(0), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "100.250.0.0", - "prefix_len": 16, + "prefix_len": float64(16), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.96.0.0", - "prefix_len": 11, + "prefix_len": float64(11), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.250.0.0", - "prefix_len": 16, + "prefix_len": float64(16), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "100.96.0.0", - "prefix_len": 11, + "prefix_len": float64(11), }, }, - }, + }), }, }, - "action": "ALLOW", }, "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", }, @@ -332,52 +331,51 @@ var _ = Describe("webhook unit test", func() { "rules": map[string]interface{}{ "policies": map[string]interface{}{ "acl-internal": map[string]interface{}{ - "permissions": []map[string]interface{}{ + "permissions": buildInterfaceSlice([]map[string]interface{}{ { "any": true, }, - }, - "principals": []map[string]interface{}{ + }), + "principals": buildInterfaceSlice([]map[string]any{ { "source_ip": map[string]interface{}{ "address_prefix": "0.0.0.0", - "prefix_len": 0, + "prefix_len": float64(0), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "100.250.0.0", - "prefix_len": 16, + "prefix_len": float64(16), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.96.0.0", - "prefix_len": 11, + "prefix_len": float64(11), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.250.0.0", - "prefix_len": 16, + "prefix_len": float64(16), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "100.96.0.0", - "prefix_len": 11, + "prefix_len": float64(11), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.9.8.7", - "prefix_len": 32, + "prefix_len": float64(32), }, }, - }, + }), }, }, - "action": "ALLOW", }, "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", }, @@ -439,34 +437,34 @@ var _ = Describe("webhook unit test", func() { "rules": map[string]interface{}{ "policies": map[string]interface{}{ "acl-internal": map[string]interface{}{ - "permissions": []map[string]interface{}{ + "permissions": buildInterfaceSlice([]map[string]interface{}{ { "any": true, }, - }, - "principals": []map[string]interface{}{ + }), + "principals": buildInterfaceSlice([]map[string]any{ { "source_ip": map[string]interface{}{ "address_prefix": "0.0.0.0", - "prefix_len": 0, + "prefix_len": float64(0), }, }, { + "remote_ip": map[string]interface{}{ "address_prefix": "100.250.0.0", - "prefix_len": 16, + "prefix_len": float64(16), }, }, { "remote_ip": map[string]interface{}{ "address_prefix": "10.96.0.0", - "prefix_len": 11, + "prefix_len": float64(11), }, }, - }, + }), }, }, - "action": "ALLOW", }, "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", }, @@ -573,3 +571,11 @@ func getEnvoyFilterFromFile(technicalID string) (filter *istionetworkingClientGo return filter } + +// buildInterfaceSlice can be used to create a slice of interface which is not possible when using literals. +func buildInterfaceSlice(maps []map[string]any) (ret []any) { + for _, m := range maps { + ret = append(ret, m) + } + return +} From fe643b98d53b7de7ddd8ac6373c660924bf0a9df Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 16:42:00 +0100 Subject: [PATCH 05/14] lowercase ruletype Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 5ebeb7391..5b1a0b7dd 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -371,7 +371,7 @@ func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoyco PrefixLen: wrapperspb.UInt32(uint32(length)), } p := new(envoyconfig_rbacv3.Principal) - switch rule.Type { + switch strings.ToLower(rule.Type) { case "source_ip": p.Identifier = &envoyconfig_rbacv3.Principal_SourceIp{SourceIp: &cidrRange} case "remote_ip": From b221b06e88dc40b6fd9551ee659a6673a7fbe2ab Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 17:05:12 +0100 Subject: [PATCH 06/14] go mod tidy Signed-off-by: Lukas Hoehl --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index 702b39309..daf65e958 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 sigs.k8s.io/controller-runtime v0.20.4 sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231015215740-bf15e44028f9 + sigs.k8s.io/yaml v1.4.0 ) require ( From 834d544a0395d2e6723854b7e46df499c5097e77 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 18 Mar 2025 17:08:13 +0100 Subject: [PATCH 07/14] format imports Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters.go | 104 +++++++++++++++---------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 5b1a0b7dd..440dd86a6 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -7,11 +7,11 @@ import ( "strings" envoy_corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" - envoyconfig_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" + envoy_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" envoy_routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" - envoyhttp_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3" - envoynetwork_rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3" - matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + envoy_httprbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3" + envoy_networkrbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3" + envoy_matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" "github.com/gardener/gardener/extensions/pkg/controller" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" @@ -38,12 +38,12 @@ type ACLRule struct { Type string `json:"type"` } -func (r *ACLRule) actionProto() envoyconfig_rbacv3.RBAC_Action { +func (r *ACLRule) actionProto() envoy_rbacv3.RBAC_Action { switch r.Action { case "DENY": - return envoyconfig_rbacv3.RBAC_DENY + return envoy_rbacv3.RBAC_DENY case "ALLOW": - return envoyconfig_rbacv3.RBAC_ALLOW + return envoy_rbacv3.RBAC_ALLOW default: panic("unknown action") } @@ -164,19 +164,19 @@ func ingressConfigPatchFromRule( rbacName := "acl-ingress" ingressSuffix := "-" + shootID + "." + seedIngressDomain - rbacFilter := &envoynetwork_rbacv3.RBAC{ + rbacFilter := &envoy_networkrbacv3.RBAC{ StatPrefix: "envoyrbac", - Rules: &envoyconfig_rbacv3.RBAC{ - Action: envoyconfig_rbacv3.RBAC_ALLOW, - Policies: map[string]*envoyconfig_rbacv3.Policy{ + Rules: &envoy_rbacv3.RBAC{ + Action: envoy_rbacv3.RBAC_ALLOW, + Policies: map[string]*envoy_rbacv3.Policy{ shootID + "-inverse": { - Permissions: []*envoyconfig_rbacv3.Permission{ + Permissions: []*envoy_rbacv3.Permission{ { - Rule: &envoyconfig_rbacv3.Permission_NotRule{ - NotRule: &envoyconfig_rbacv3.Permission{ - Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ - RequestedServerName: &matcherv3.StringMatcher{ - MatchPattern: &matcherv3.StringMatcher_Suffix{ + Rule: &envoy_rbacv3.Permission_NotRule{ + NotRule: &envoy_rbacv3.Permission{ + Rule: &envoy_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &envoy_matcherv3.StringMatcher{ + MatchPattern: &envoy_matcherv3.StringMatcher_Suffix{ Suffix: ingressSuffix, }, }, @@ -185,9 +185,9 @@ func ingressConfigPatchFromRule( }, }, }, - Principals: []*envoyconfig_rbacv3.Principal{ + Principals: []*envoy_rbacv3.Principal{ { - Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + Identifier: &envoy_rbacv3.Principal_RemoteIp{ RemoteIp: &envoy_corev3.CidrRange{ AddressPrefix: "0.0.0.0", PrefixLen: wrapperspb.UInt32(0), @@ -197,11 +197,11 @@ func ingressConfigPatchFromRule( }, }, shootID: { - Permissions: []*envoyconfig_rbacv3.Permission{ + Permissions: []*envoy_rbacv3.Permission{ { - Rule: &envoyconfig_rbacv3.Permission_RequestedServerName{ - RequestedServerName: &matcherv3.StringMatcher{ - MatchPattern: &matcherv3.StringMatcher_Suffix{ + Rule: &envoy_rbacv3.Permission_RequestedServerName{ + RequestedServerName: &envoy_matcherv3.StringMatcher{ + MatchPattern: &envoy_matcherv3.StringMatcher_Suffix{ Suffix: ingressSuffix, }, }, @@ -250,8 +250,8 @@ func vpnConfigPatchFromRule(rule *ACLRule, headerMatcher := envoy_routev3.HeaderMatcher{ Name: "reversed-vpn", HeaderMatchSpecifier: &envoy_routev3.HeaderMatcher_StringMatch{ - StringMatch: &matcherv3.StringMatcher{ - MatchPattern: &matcherv3.StringMatcher_Contains{ + StringMatch: &envoy_matcherv3.StringMatcher{ + MatchPattern: &envoy_matcherv3.StringMatcher_Contains{ // The actual header value will look something like // `outbound|1194||vpn-seed-server..svc.cluster.local`. // Include dots in the contains matcher as anchors, to always match the entire technical shoot ID. @@ -265,26 +265,26 @@ func vpnConfigPatchFromRule(rule *ACLRule, }, } - rbacFilter := &envoyhttp_rbacv3.RBAC{ + rbacFilter := &envoy_httprbacv3.RBAC{ RulesStatPrefix: "envoyrbac", - Rules: &envoyconfig_rbacv3.RBAC{ - Action: envoyconfig_rbacv3.RBAC_ALLOW, - Policies: map[string]*envoyconfig_rbacv3.Policy{ + Rules: &envoy_rbacv3.RBAC{ + Action: envoy_rbacv3.RBAC_ALLOW, + Policies: map[string]*envoy_rbacv3.Policy{ shortShootID + "-inverse": { - Permissions: []*envoyconfig_rbacv3.Permission{ + Permissions: []*envoy_rbacv3.Permission{ { - Rule: &envoyconfig_rbacv3.Permission_NotRule{ - NotRule: &envoyconfig_rbacv3.Permission{ - Rule: &envoyconfig_rbacv3.Permission_Header{ + Rule: &envoy_rbacv3.Permission_NotRule{ + NotRule: &envoy_rbacv3.Permission{ + Rule: &envoy_rbacv3.Permission_Header{ Header: &headerMatcher, }, }, }, }, }, - Principals: []*envoyconfig_rbacv3.Principal{ + Principals: []*envoy_rbacv3.Principal{ { - Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + Identifier: &envoy_rbacv3.Principal_RemoteIp{ RemoteIp: &envoy_corev3.CidrRange{ AddressPrefix: "0.0.0.0", PrefixLen: wrapperspb.UInt32(0), @@ -294,9 +294,9 @@ func vpnConfigPatchFromRule(rule *ACLRule, }, }, shortShootID: { - Permissions: []*envoyconfig_rbacv3.Permission{ + Permissions: []*envoy_rbacv3.Permission{ { - Rule: &envoyconfig_rbacv3.Permission_Header{ + Rule: &envoy_rbacv3.Permission_Header{ Header: &headerMatcher, }, }, @@ -358,8 +358,8 @@ func CreateInternalFilterPatchFromRule( // into a list of envoy principals. The function checks for the rule action: If // the action is "ALLOW", the alwaysAllowedCIDRs are appended to the principals // to guarantee the downstream flow for these CIDRs is not blocked. -func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoyconfig_rbacv3.Principal { - principals := []*envoyconfig_rbacv3.Principal{} +func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoy_rbacv3.Principal { + principals := []*envoy_rbacv3.Principal{} for _, cidr := range rule.Cidrs { prefix, length, err := getPrefixAndPrefixLength(cidr) @@ -370,14 +370,14 @@ func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoyco AddressPrefix: prefix, PrefixLen: wrapperspb.UInt32(uint32(length)), } - p := new(envoyconfig_rbacv3.Principal) + p := new(envoy_rbacv3.Principal) switch strings.ToLower(rule.Type) { case "source_ip": - p.Identifier = &envoyconfig_rbacv3.Principal_SourceIp{SourceIp: &cidrRange} + p.Identifier = &envoy_rbacv3.Principal_SourceIp{SourceIp: &cidrRange} case "remote_ip": - p.Identifier = &envoyconfig_rbacv3.Principal_RemoteIp{RemoteIp: &cidrRange} + p.Identifier = &envoy_rbacv3.Principal_RemoteIp{RemoteIp: &cidrRange} case "direct_remote_ip": - p.Identifier = &envoyconfig_rbacv3.Principal_DirectRemoteIp{DirectRemoteIp: &cidrRange} + p.Identifier = &envoy_rbacv3.Principal_DirectRemoteIp{DirectRemoteIp: &cidrRange} default: continue } @@ -393,8 +393,8 @@ func ruleCIDRsToPrincipal(rule *ACLRule, alwaysAllowedCIDRs []string) []*envoyco if err != nil { continue } - principals = append(principals, &envoyconfig_rbacv3.Principal{ - Identifier: &envoyconfig_rbacv3.Principal_RemoteIp{ + principals = append(principals, &envoy_rbacv3.Principal{ + Identifier: &envoy_rbacv3.Principal_RemoteIp{ RemoteIp: &envoy_corev3.CidrRange{ AddressPrefix: prefix, PrefixLen: wrapperspb.UInt32(uint32(length)), @@ -420,7 +420,7 @@ func getPrefixAndPrefixLength(cidr string) (prefix string, prefixLen int, err er } func principalsToPatch( - rbacName string, ruleAction envoyconfig_rbacv3.RBAC_Action, principals []*envoyconfig_rbacv3.Principal, + rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal, ) *istio_networkingv1alpha3.EnvoyFilter_Patch { rbacFilter := newRBACFilter(rbacName, ruleAction, principals) typedConfig, err := protoMessageToTypedConfig(rbacFilter) @@ -438,16 +438,16 @@ func principalsToPatch( } } -func newRBACFilter(rbacName string, ruleAction envoyconfig_rbacv3.RBAC_Action, principals []*envoyconfig_rbacv3.Principal) *envoynetwork_rbacv3.RBAC { - return &envoynetwork_rbacv3.RBAC{ +func newRBACFilter(rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal) *envoy_networkrbacv3.RBAC { + return &envoy_networkrbacv3.RBAC{ StatPrefix: "envoyrbac", - Rules: &envoyconfig_rbacv3.RBAC{ + Rules: &envoy_rbacv3.RBAC{ Action: ruleAction, - Policies: map[string]*envoyconfig_rbacv3.Policy{ + Policies: map[string]*envoy_rbacv3.Policy{ rbacName: { - Permissions: []*envoyconfig_rbacv3.Permission{ + Permissions: []*envoy_rbacv3.Permission{ { - Rule: &envoyconfig_rbacv3.Permission_Any{ + Rule: &envoy_rbacv3.Permission_Any{ Any: true, }, }, From 4334eb1e74de0a4e48aace9bcc16aa424c8bc2e7 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 15 Apr 2025 17:02:44 +0200 Subject: [PATCH 08/14] doc: asStructPB Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 440dd86a6..451d4ff07 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -56,7 +56,7 @@ type FilterPatch struct { TypedConfig *structpb.Struct `json:"typed_config"` } -// AsStructPB returns FilterPatch represented as a structpb.Struct +// asStructPB returns FilterPatch represented as a structpb.Struct func (f *FilterPatch) asStructPB() *structpb.Struct { pb, err := structpb.NewStruct(map[string]any{ "name": f.Name, From 8ade0c57411e3122c5f671ae3941fa93d223919a Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 15 Apr 2025 17:06:27 +0200 Subject: [PATCH 09/14] move code to be structured better Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters.go | 122 +++++++++++++++---------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 451d4ff07..337bb5cea 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -114,48 +114,6 @@ func BuildIngressEnvoyFilterSpecForHelmChart( return nil } -// BuildVPNEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for VPN. -func BuildVPNEnvoyFilterSpecForHelmChart( - cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) *istio_networkingv1alpha3.EnvoyFilter { - return &istio_networkingv1alpha3.EnvoyFilter{ - WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ - Labels: istioLabels, - }, - ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ - vpnConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs), - }, - } -} - -// CreateAPIConfigPatchFromRule combines an ACLRule, the first entry of the -// hosts list and the alwaysAllowedCIDRs into a network filter patch that can be -// applied to the `GATEWAY` network filter chain matching the host. -func CreateAPIConfigPatchFromRule( - rule *ACLRule, hosts, alwaysAllowedCIDRs []string, -) (*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { - if len(hosts) == 0 { - return nil, ErrNoHostsGiven - } - rbacName := "acl-api" - principals := ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs) - - return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ - ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, - Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ - Context: istio_networkingv1alpha3.EnvoyFilter_GATEWAY, - ObjectTypes: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Listener{ - Listener: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch{ - FilterChain: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch_FilterChainMatch{ - Sni: hosts[0], - }, - }, - }, - }, - Patch: principalsToPatch(rbacName, rule.actionProto(), principals), - }, nil -} - // ingressConfigPatchFromRule creates a network filter patch that can be // applied to the `GATEWAY` network filter chain matching the wildcard ingress domain. func ingressConfigPatchFromRule( @@ -241,6 +199,20 @@ func ingressConfigPatchFromRule( } } +// BuildVPNEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for VPN. +func BuildVPNEnvoyFilterSpecForHelmChart( + cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, +) *istio_networkingv1alpha3.EnvoyFilter { + return &istio_networkingv1alpha3.EnvoyFilter{ + WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ + Labels: istioLabels, + }, + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + vpnConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs), + }, + } +} + // vpnConfigPatchFromRule creates an HTTP filter patch that can be applied to the // `GATEWAY` HTTP filter chain for the VPN. func vpnConfigPatchFromRule(rule *ACLRule, @@ -332,6 +304,53 @@ func vpnConfigPatchFromRule(rule *ACLRule, } } +// CreateAPIConfigPatchFromRule combines an ACLRule, the first entry of the +// hosts list and the alwaysAllowedCIDRs into a network filter patch that can be +// applied to the `GATEWAY` network filter chain matching the host. +func CreateAPIConfigPatchFromRule( + rule *ACLRule, hosts, alwaysAllowedCIDRs []string, +) (*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { + if len(hosts) == 0 { + return nil, ErrNoHostsGiven + } + rbacName := "acl-api" + principals := ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs) + + return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ + ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, + Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ + Context: istio_networkingv1alpha3.EnvoyFilter_GATEWAY, + ObjectTypes: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Listener{ + Listener: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch{ + FilterChain: &istio_networkingv1alpha3.EnvoyFilter_ListenerMatch_FilterChainMatch{ + Sni: hosts[0], + }, + }, + }, + }, + Patch: principalsToPatch(rbacName, rule.actionProto(), principals), + }, nil +} + +func principalsToPatch( + rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal, +) *istio_networkingv1alpha3.EnvoyFilter_Patch { + rbacFilter := newRBACFilter(rbacName, ruleAction, principals) + typedConfig, err := protoMessageToTypedConfig(rbacFilter) + if err != nil { + // this is a error in the code itself, don't return to caller + panic(err) + } + filter := &FilterPatch{ + Name: rbacName, + TypedConfig: typedConfig, + } + return &istio_networkingv1alpha3.EnvoyFilter_Patch{ + Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, + Value: filter.asStructPB(), + } +} + // CreateInternalFilterPatchFromRule combines an ACLRule, the // alwaysAllowedCIDRs, and the shootSpecificCIDRs into a filter patch. func CreateInternalFilterPatchFromRule( @@ -419,25 +438,6 @@ func getPrefixAndPrefixLength(cidr string) (prefix string, prefixLen int, err er return ip.String(), prefixLen, nil } -func principalsToPatch( - rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal, -) *istio_networkingv1alpha3.EnvoyFilter_Patch { - rbacFilter := newRBACFilter(rbacName, ruleAction, principals) - typedConfig, err := protoMessageToTypedConfig(rbacFilter) - if err != nil { - // this is a error in the code itself, don't return to caller - panic(err) - } - filter := &FilterPatch{ - Name: rbacName, - TypedConfig: typedConfig, - } - return &istio_networkingv1alpha3.EnvoyFilter_Patch{ - Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, - Value: filter.asStructPB(), - } -} - func newRBACFilter(rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal) *envoy_networkrbacv3.RBAC { return &envoy_networkrbacv3.RBAC{ StatPrefix: "envoyrbac", From af7f1ecc205c5400bee29bd4585261fe119b9c14 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 15 Apr 2025 17:14:40 +0200 Subject: [PATCH 10/14] move findTCPProxyFilter into function Signed-off-by: Lukas Hoehl --- pkg/webhook/webhook.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 572007cda..7e5054640 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -117,7 +117,19 @@ func (e *EnvoyFilterWebhook) createAdmissionResponse( shootSpecificCIRDs = append(shootSpecificCIRDs, providerSpecificCIRDs...) } - var originalFilter *structpb.Struct + originalFilter := findTCPProxyFilter(filter) + if originalFilter == nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("filter with name \"envoy.filters.network.tcp_proxy\" not present in EnvoyFilter %s/%s", filter.Namespace, filter.Name)) + } + filterPatch := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) + + // make sure the original filter is the last + filterPatches := []map[string]interface{}{filterPatch.AsMap(), originalFilter.AsMap()} + + return buildAdmissionResponseWithFilterPatches(filterPatches) +} + +func findTCPProxyFilter(filter *istionetworkingClientGo.EnvoyFilter) *structpb.Struct { for _, configpatch := range filter.Spec.ConfigPatches { patch := configpatch.Patch.Value filters, ok := patch.Fields["filters"] @@ -138,20 +150,11 @@ func (e *EnvoyFilterWebhook) createAdmissionResponse( continue } if name.GetStringValue() == "envoy.filters.network.tcp_proxy" { - originalFilter = structVal - break + return structVal } } } - if originalFilter == nil { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("filter with name \"envoy.filters.network.tcp_proxy\" not present in EnvoyFilter %s/%s", filter.Namespace, filter.Name)) - } - filterPatch := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) - - // make sure the original filter is the last - filterPatches := []map[string]interface{}{filterPatch.AsMap(), originalFilter.AsMap()} - - return buildAdmissionResponseWithFilterPatches(filterPatches) + return nil } func buildAdmissionResponseWithFilterPatches(filters []map[string]interface{}) admission.Response { From e1e3d170d269d52c6f967d0936e18e0e66928cbe Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 15 Apr 2025 17:34:06 +0200 Subject: [PATCH 11/14] remove panics Signed-off-by: Lukas Hoehl --- pkg/controller/actuator.go | 15 +++- pkg/envoyfilters/envoyfilters.go | 125 ++++++++++++++++---------- pkg/envoyfilters/envoyfilters_test.go | 14 ++- pkg/webhook/webhook.go | 11 ++- 4 files changed, 106 insertions(+), 59 deletions(-) diff --git a/pkg/controller/actuator.go b/pkg/controller/actuator.go index 9fe94b1f2..2aeae16c6 100644 --- a/pkg/controller/actuator.go +++ b/pkg/controller/actuator.go @@ -276,9 +276,12 @@ func (a *actuator) createSeedResources( return err } - vpnEnvoyFilterSpec := envoyfilters.BuildVPNEnvoyFilterSpecForHelmChart( + vpnEnvoyFilterSpec, err := envoyfilters.BuildVPNEnvoyFilterSpecForHelmChart( cluster, spec.Rule, alwaysAllowedCIDRs, istioLabels, ) + if err != nil { + return err + } cfg := map[string]interface{}{ "shootName": cluster.Shoot.Status.TechnicalID, @@ -294,10 +297,14 @@ func (a *actuator) createSeedResources( // The `nginx-ingress-controller` Gateway object only exists in g/g@v1.89, (introduced with // https://github.com/gardener/gardener/pull/9038). // If it doesn't exist yet, we can't apply ACLs to shoot ingresses. - ingressEnvoyFilterSpec := envoyfilters.BuildIngressEnvoyFilterSpecForHelmChart( + ingressEnvoyFilterSpec, err := envoyfilters.BuildIngressEnvoyFilterSpecForHelmChart( cluster, spec.Rule, alwaysAllowedCIDRs, defaultLabels) - - cfg["ingressEnvoyFilterSpec"] = ingressEnvoyFilterSpec + if err != nil { + return err + } + if ingressEnvoyFilterSpec != nil { + cfg["ingressEnvoyFilterSpec"] = ingressEnvoyFilterSpec + } } cfg, err = chart.InjectImages(cfg, imagevector.ImageVector(), []string{ImageName}) diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go index 337bb5cea..9fff0daf1 100644 --- a/pkg/envoyfilters/envoyfilters.go +++ b/pkg/envoyfilters/envoyfilters.go @@ -38,14 +38,14 @@ type ACLRule struct { Type string `json:"type"` } -func (r *ACLRule) actionProto() envoy_rbacv3.RBAC_Action { +func (r *ACLRule) actionProto() (envoy_rbacv3.RBAC_Action, error) { switch r.Action { case "DENY": - return envoy_rbacv3.RBAC_DENY + return envoy_rbacv3.RBAC_DENY, nil case "ALLOW": - return envoy_rbacv3.RBAC_ALLOW + return envoy_rbacv3.RBAC_ALLOW, nil default: - panic("unknown action") + return -1, fmt.Errorf("unknown action %s", r.Action) } } @@ -57,21 +57,24 @@ type FilterPatch struct { } // asStructPB returns FilterPatch represented as a structpb.Struct -func (f *FilterPatch) asStructPB() *structpb.Struct { +func (f *FilterPatch) asStructPB() (*structpb.Struct, error) { pb, err := structpb.NewStruct(map[string]any{ "name": f.Name, "typed_config": f.TypedConfig.AsMap(), }) if err != nil { - // This state is not valid and should not be propergated - panic(err) + return nil, err } - return pb + return pb, nil } // AsMap returns FilterPatch represented as a map[string]interface{} -func (f *FilterPatch) AsMap() map[string]interface{} { - return f.asStructPB().AsMap() +func (f *FilterPatch) AsMap() (map[string]interface{}, error) { + s, err := f.asStructPB() + if err != nil { + return nil, err + } + return s.AsMap(), nil } // BuildAPIEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for API server @@ -97,28 +100,30 @@ func BuildAPIEnvoyFilterSpecForHelmChart( // endpoints using the seed ingress domain. func BuildIngressEnvoyFilterSpecForHelmChart( cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) *istio_networkingv1alpha3.EnvoyFilter { +) (*istio_networkingv1alpha3.EnvoyFilter, error) { seedIngressDomain := helper.GetSeedIngressDomain(cluster.Seed) - if seedIngressDomain != "" { - shootID := helper.ComputeShortShootID(cluster.Shoot) + if seedIngressDomain == "" { + return nil, nil + } + shootID := helper.ComputeShortShootID(cluster.Shoot) - return &istio_networkingv1alpha3.EnvoyFilter{ - WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ - Labels: istioLabels, - }, - ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ - ingressConfigPatchFromRule(rule, seedIngressDomain, shootID, alwaysAllowedCIDRs), - }, - } + configPatch, err := ingressConfigPatchFromRule(rule, seedIngressDomain, shootID, alwaysAllowedCIDRs) + if err != nil { + return nil, err } - return nil + return &istio_networkingv1alpha3.EnvoyFilter{ + WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ + Labels: istioLabels, + }, + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{configPatch}, + }, nil } // ingressConfigPatchFromRule creates a network filter patch that can be // applied to the `GATEWAY` network filter chain matching the wildcard ingress domain. func ingressConfigPatchFromRule( rule *ACLRule, seedIngressDomain, shootID string, alwaysAllowedCIDRs []string, -) *istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch { +) (*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { rbacName := "acl-ingress" ingressSuffix := "-" + shootID + "." + seedIngressDomain @@ -173,13 +178,16 @@ func ingressConfigPatchFromRule( } typedConfig, err := protoMessageToTypedConfig(rbacFilter) if err != nil { - // this is a error in the code itself, don't return to caller - panic(err) + return nil, err } filter := &FilterPatch{ Name: rbacName, TypedConfig: typedConfig, } + pb, err := filter.asStructPB() + if err != nil { + return nil, err + } return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ @@ -194,30 +202,32 @@ func ingressConfigPatchFromRule( }, Patch: &istio_networkingv1alpha3.EnvoyFilter_Patch{ Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, - Value: filter.asStructPB(), + Value: pb, }, - } + }, nil } // BuildVPNEnvoyFilterSpecForHelmChart assembles EnvoyFilter patches for VPN. func BuildVPNEnvoyFilterSpecForHelmChart( cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string, -) *istio_networkingv1alpha3.EnvoyFilter { +) (*istio_networkingv1alpha3.EnvoyFilter, error) { + patch, err := vpnConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs) + if err != nil { + return nil, err + } return &istio_networkingv1alpha3.EnvoyFilter{ WorkloadSelector: &istio_networkingv1alpha3.WorkloadSelector{ Labels: istioLabels, }, - ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ - vpnConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs), - }, - } + ConfigPatches: []*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{patch}, + }, nil } // vpnConfigPatchFromRule creates an HTTP filter patch that can be applied to the // `GATEWAY` HTTP filter chain for the VPN. func vpnConfigPatchFromRule(rule *ACLRule, shortShootID, technicalShootID string, alwaysAllowedCIDRs []string, -) *istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch { +) (*istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { rbacName := "acl-vpn" headerMatcher := envoy_routev3.HeaderMatcher{ Name: "reversed-vpn", @@ -280,13 +290,17 @@ func vpnConfigPatchFromRule(rule *ACLRule, } typedConfig, err := protoMessageToTypedConfig(rbacFilter) if err != nil { - // this is a error in the code itself, don't return to caller - panic(err) + return nil, err } filterPatch := &FilterPatch{ Name: rbacName, TypedConfig: typedConfig, } + pb, err := filterPatch.asStructPB() + if err != nil { + return nil, err + } + return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ ApplyTo: istio_networkingv1alpha3.EnvoyFilter_HTTP_FILTER, Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ @@ -299,9 +313,9 @@ func vpnConfigPatchFromRule(rule *ACLRule, }, Patch: &istio_networkingv1alpha3.EnvoyFilter_Patch{ Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, - Value: filterPatch.asStructPB(), + Value: pb, }, - } + }, nil } // CreateAPIConfigPatchFromRule combines an ACLRule, the first entry of the @@ -315,7 +329,14 @@ func CreateAPIConfigPatchFromRule( } rbacName := "acl-api" principals := ruleCIDRsToPrincipal(rule, alwaysAllowedCIDRs) - + action, err := rule.actionProto() + if err != nil { + return nil, err + } + patch, err := principalsToPatch(rbacName, action, principals) + if err != nil { + return nil, err + } return &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{ ApplyTo: istio_networkingv1alpha3.EnvoyFilter_NETWORK_FILTER, Match: &istio_networkingv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ @@ -328,27 +349,30 @@ func CreateAPIConfigPatchFromRule( }, }, }, - Patch: principalsToPatch(rbacName, rule.actionProto(), principals), + Patch: patch, }, nil } func principalsToPatch( rbacName string, ruleAction envoy_rbacv3.RBAC_Action, principals []*envoy_rbacv3.Principal, -) *istio_networkingv1alpha3.EnvoyFilter_Patch { +) (*istio_networkingv1alpha3.EnvoyFilter_Patch, error) { rbacFilter := newRBACFilter(rbacName, ruleAction, principals) typedConfig, err := protoMessageToTypedConfig(rbacFilter) if err != nil { - // this is a error in the code itself, don't return to caller - panic(err) + return nil, err } filter := &FilterPatch{ Name: rbacName, TypedConfig: typedConfig, } + pb, err := filter.asStructPB() + if err != nil { + return nil, err + } return &istio_networkingv1alpha3.EnvoyFilter_Patch{ Operation: istio_networkingv1alpha3.EnvoyFilter_Patch_INSERT_FIRST, - Value: filter.asStructPB(), - } + Value: pb, + }, nil } // CreateInternalFilterPatchFromRule combines an ACLRule, the @@ -357,20 +381,23 @@ func CreateInternalFilterPatchFromRule( rule *ACLRule, alwaysAllowedCIDRs []string, shootSpecificCIDRs []string, -) *FilterPatch { +) (*FilterPatch, error) { rbacName := "acl-internal" principals := ruleCIDRsToPrincipal(rule, append(alwaysAllowedCIDRs, shootSpecificCIDRs...)) - rbacFilter := newRBACFilter(rbacName, rule.actionProto(), principals) + action, err := rule.actionProto() + if err != nil { + return nil, err + } + rbacFilter := newRBACFilter(rbacName, action, principals) typedConfig, err := protoMessageToTypedConfig(rbacFilter) if err != nil { - // this is a error in the code itself, don't return to caller - panic(err) + return nil, err } return &FilterPatch{ Name: rbacName + "-" + strings.ToLower(rule.Type), TypedConfig: typedConfig, - } + }, nil } // ruleCIDRsToPrincipal translates a list of strings in the form "0.0.0.0/0" diff --git a/pkg/envoyfilters/envoyfilters_test.go b/pkg/envoyfilters/envoyfilters_test.go index 67d0df8af..df03e9939 100644 --- a/pkg/envoyfilters/envoyfilters_test.go +++ b/pkg/envoyfilters/envoyfilters_test.go @@ -68,7 +68,8 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { "app": "istio-ingressgateway", "istio": "ingressgateway", } - ingressEnvoyFilterSpec := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + ingressEnvoyFilterSpec, err := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + Expect(err).NotTo(HaveOccurred()) checkIfFilterEquals(ingressEnvoyFilterSpec, "ingressEnvoyFilterSpecWithOneAllowRule.yaml") }) @@ -79,7 +80,9 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { "app": "istio-ingressgateway", "istio": "ingressgateway", } - ingressEnvoyFilterSpec := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + ingressEnvoyFilterSpec, err := BuildIngressEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + + Expect(err).NotTo(HaveOccurred()) Expect(ingressEnvoyFilterSpec).To(BeNil()) }) }) @@ -93,7 +96,9 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { "app": "istio-ingressgateway", "istio": "ingressgateway", } - result := BuildVPNEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + result, err := BuildVPNEnvoyFilterSpecForHelmChart(cluster, rule, alwaysAllowedCIDRs, labels) + + Expect(err).NotTo(HaveOccurred()) checkIfFilterEquals(result, "vpnEnvoyFilterSpecWithOneAllowRule.yaml") }) }) @@ -104,7 +109,8 @@ var _ = Describe("EnvoyFilter Unit Tests", func() { It("Should create a filter spec matching the expected one, including the always allowed CIDRs", func() { rule := createRule("ALLOW", "remote_ip", "0.0.0.0/0") - result := CreateInternalFilterPatchFromRule(rule, alwaysAllowedCIDRs, []string{}) + result, err := CreateInternalFilterPatchFromRule(rule, alwaysAllowedCIDRs, []string{}) + Expect(err).NotTo(HaveOccurred()) checkIfFilterEquals(result, "singleFiltersAllowEntry.yaml") }) }) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 7e5054640..18392d2f8 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -121,10 +121,17 @@ func (e *EnvoyFilterWebhook) createAdmissionResponse( if originalFilter == nil { return admission.Errored(http.StatusBadRequest, fmt.Errorf("filter with name \"envoy.filters.network.tcp_proxy\" not present in EnvoyFilter %s/%s", filter.Namespace, filter.Name)) } - filterPatch := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) + filterPatch, err := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + filterPatchMap, err := filterPatch.AsMap() + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } // make sure the original filter is the last - filterPatches := []map[string]interface{}{filterPatch.AsMap(), originalFilter.AsMap()} + filterPatches := []map[string]interface{}{filterPatchMap, originalFilter.AsMap()} return buildAdmissionResponseWithFilterPatches(filterPatches) } From 6848f285d0b9b07db1e4ce7ca66ba70c064b97c8 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Tue, 15 Apr 2025 17:36:22 +0200 Subject: [PATCH 12/14] doc: checkIfFilterEquals Signed-off-by: Lukas Hoehl --- pkg/envoyfilters/envoyfilters_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/envoyfilters/envoyfilters_test.go b/pkg/envoyfilters/envoyfilters_test.go index df03e9939..8c0b88fd8 100644 --- a/pkg/envoyfilters/envoyfilters_test.go +++ b/pkg/envoyfilters/envoyfilters_test.go @@ -141,7 +141,7 @@ func createRule(action, ruleType, cidr string) *ACLRule { } } -// checkIfFilterEqualsYAML takes a map as input, and tries to compare its +// checkIfFilterEquals takes an object as input, and tries to compare its // marshaled contents to the string coming from the specified testdata file. // Fails the test if strings differ. The file contents are unmarshaled and // marshaled again to guarantee the strings are comparable. From 56e322ff12f4c324e401a5d44ba7ea926e10b0c9 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Thu, 7 Aug 2025 11:07:32 +0200 Subject: [PATCH 13/14] go mod tidy Signed-off-by: Lukas Hoehl --- go.mod | 29 ++++++++++++++--------------- go.sum | 16 ++++++++-------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index daf65e958..98711922a 100644 --- a/go.mod +++ b/go.mod @@ -6,10 +6,10 @@ toolchain go1.24.6 require ( github.com/ahmetb/gen-crd-api-reference-docs v0.3.0 - github.com/envoyproxy/go-control-plane/envoy v1.32.3 - github.com/gardener/gardener v1.112.0 - github.com/gardener/gardener-extension-provider-openstack v1.41.2 - github.com/go-logr/logr v1.4.2 + github.com/envoyproxy/go-control-plane v0.13.1 + github.com/gardener/gardener v1.117.6 + github.com/gardener/gardener-extension-provider-openstack v1.47.0 + github.com/go-logr/logr v1.4.3 github.com/golang/mock v1.6.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 @@ -17,7 +17,8 @@ require ( github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.7 golang.org/x/tools v0.34.0 - gopkg.in/yaml.v3 v3.0.1 + gomodules.xyz/jsonpatch/v2 v2.5.0 + google.golang.org/protobuf v1.36.5 istio.io/api v1.25.2 istio.io/client-go v1.25.1 k8s.io/api v0.32.7 @@ -26,8 +27,6 @@ require ( k8s.io/client-go v0.32.7 k8s.io/code-generator v0.32.7 k8s.io/component-base v0.32.7 - gomodules.xyz/jsonpatch/v2 v2.4.0 - google.golang.org/protobuf v1.36.5 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 sigs.k8s.io/controller-runtime v0.20.4 sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231015215740-bf15e44028f9 @@ -35,7 +34,7 @@ require ( ) require ( - cel.dev/expr v0.16.1 // indirect + cel.dev/expr v0.19.0 // indirect dario.cat/mergo v1.0.1 // indirect github.com/BurntSushi/toml v1.4.0 // indirect github.com/Masterminds/goutils v1.1.1 // indirect @@ -50,7 +49,7 @@ require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect github.com/envoyproxy/protoc-gen-validate v1.1.0 // indirect - github.com/evanphx/json-patch/v5 v5.9.0 // indirect + github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/fatih/color v1.18.0 // indirect github.com/fluent/fluent-operator/v3 v3.3.0 // indirect github.com/fsnotify/fsnotify v1.8.0 // indirect @@ -95,15 +94,15 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect - github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.78.2 // indirect - github.com/prometheus/client_golang v1.20.5 // indirect + github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.81.0 // indirect + github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.63.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/shopspring/decimal v1.4.0 // indirect - github.com/spf13/afero v1.12.0 // indirect - github.com/spf13/cast v1.7.0 // indirect + github.com/spf13/afero v1.14.0 // indirect + github.com/spf13/cast v1.7.1 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect @@ -122,11 +121,12 @@ require ( golang.org/x/term v0.32.0 // indirect golang.org/x/text v0.26.0 // indirect golang.org/x/time v0.11.0 // indirect - gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8 // indirect + gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect helm.sh/helm/v3 v3.17.3 // indirect k8s.io/autoscaler v0.0.0-20190805135949-100e91ba756e // indirect k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 // indirect @@ -140,5 +140,4 @@ require ( sigs.k8s.io/controller-tools v0.17.3 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.5.0 // indirect - sigs.k8s.io/yaml v1.4.0 // indirect ) diff --git a/go.sum b/go.sum index beb4d37b2..47c634750 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -cel.dev/expr v0.16.1 h1:NR0+oFYzR1CqLFhTAqg3ql59G9VfN8fKq1TCHJ6gq1g= -cel.dev/expr v0.16.1/go.mod h1:AsGA5zb3WruAEQeQng1RZdGEXmBj0jvMWh6l5SnNuC8= +cel.dev/expr v0.19.0 h1:lXuo+nDhpyJSpWxpPVi5cPUwzKb+dsdOiw6IreM5yt0= +cel.dev/expr v0.19.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= @@ -80,8 +80,8 @@ github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= -github.com/envoyproxy/go-control-plane/envoy v1.32.3 h1:hVEaommgvzTjTd4xCaFd+kEQ2iYBtGxP6luyLrx6uOk= -github.com/envoyproxy/go-control-plane/envoy v1.32.3/go.mod h1:F6hWupPfh75TBXGKA++MCT/CZHFq5r9/uwt/kQYkZfE= +github.com/envoyproxy/go-control-plane v0.13.1 h1:vPfJZCkob6yTMEgS+0TwfTUfbHjfy/6vOJ8hUWX/uXE= +github.com/envoyproxy/go-control-plane v0.13.1/go.mod h1:X45hY0mufo6Fd0KW3rqsGvQMw58jvjymeCzBU3mWyHw= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v1.1.0 h1:tntQDh69XqOCOZsDz0lVJQez/2L6Uu2PdjCQwWCJ3bM= github.com/envoyproxy/protoc-gen-validate v1.1.0/go.mod h1:sXRDRVmzEbkM7CVcM06s9shE/m23dg3wzjl0UWqJ2q4= @@ -588,10 +588,10 @@ honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= -istio.io/api v1.25.0 h1:UVQNFUAz4R06NagA+QpEzLmi4sw+m+UuMgD6sIyXawA= -istio.io/api v1.25.0/go.mod h1:QFzEXv/IT582T0FHZVp1QoolvE4ws0zz/vVO55blmlE= -istio.io/client-go v1.24.2 h1:JTTfBV6dv+AAW+AfccyrdX4T1f9CpsXd1Yzo1s/IYAI= -istio.io/client-go v1.24.2/go.mod h1:dgZ9EmJzh1EECzf6nQhwNL4R6RvlyeH/RXeNeNp/MRg= +istio.io/api v1.25.2 h1:FCRQy7iaTreKJdLemlQ1vRJEsf1soCHoTAuSf68w5I8= +istio.io/api v1.25.2/go.mod h1:QFzEXv/IT582T0FHZVp1QoolvE4ws0zz/vVO55blmlE= +istio.io/client-go v1.25.1 h1:1Asibz5v2NBA6w4Sqa85NS7TkpEolZcPKAT5oUAXWi8= +istio.io/client-go v1.25.1/go.mod h1:Vap9OyHJMvvDegYoZczcNybS4wbPaTk+4bZcWMb8+vE= k8s.io/api v0.19.0/go.mod h1:I1K45XlvTrDjmj5LoM5LuP/KYrhWbjUKT/SoPG0qTjw= k8s.io/api v0.32.7 h1:CBhHkoi3YJW8QQI6VL/Hu9f1HHVImmuIh513d4H4VfQ= k8s.io/api v0.32.7/go.mod h1:YEB46LZ/M0/9t0m+R2FxW5fkZAUR/eoS6sZQKS3mBYk= From 3c0bb13f4a8e15d19595f18d55ca398e6f64c2c5 Mon Sep 17 00:00:00 2001 From: Lukas Hoehl Date: Thu, 7 Aug 2025 11:08:01 +0200 Subject: [PATCH 14/14] drop webhook Signed-off-by: Lukas Hoehl --- go.mod | 2 +- pkg/webhook/webhook.go | 181 ----------- pkg/webhook/webhook_test.go | 581 ------------------------------------ 3 files changed, 1 insertion(+), 763 deletions(-) delete mode 100644 pkg/webhook/webhook.go delete mode 100644 pkg/webhook/webhook_test.go diff --git a/go.mod b/go.mod index 98711922a..18cc8b3df 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.7 golang.org/x/tools v0.34.0 - gomodules.xyz/jsonpatch/v2 v2.5.0 google.golang.org/protobuf v1.36.5 istio.io/api v1.25.2 istio.io/client-go v1.25.1 @@ -121,6 +120,7 @@ require ( golang.org/x/term v0.32.0 // indirect golang.org/x/text v0.26.0 // indirect golang.org/x/time v0.11.0 // indirect + gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go deleted file mode 100644 index 18392d2f8..000000000 --- a/pkg/webhook/webhook.go +++ /dev/null @@ -1,181 +0,0 @@ -package webhook - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "strings" - - v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" - v1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper" - extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - "gomodules.xyz/jsonpatch/v2" - "google.golang.org/protobuf/types/known/structpb" - istionetworkingClientGo "istio.io/client-go/pkg/apis/networking/v1alpha3" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/stackitcloud/gardener-extension-acl/pkg/controller" - "github.com/stackitcloud/gardener-extension-acl/pkg/envoyfilters" - "github.com/stackitcloud/gardener-extension-acl/pkg/extensionspec" - "github.com/stackitcloud/gardener-extension-acl/pkg/helper" -) - -const ( - // ExtensionName contains the ACl extension name. - ExtensionName = "acl" -) - -// EnvoyFilterWebhook is a service struct that defines functions to handle -// admission requests for EnvoyFilters. -type EnvoyFilterWebhook struct { - Client client.Client - Decoder admission.Decoder - AdditionalAllowedCIDRs []string -} - -// Handle receives incoming admission requests for EnvoyFilters and returns a -// response. The response contains patches for specific EnvoyFilters, which need -// to be patched for the ACL extension to be able to control all filter chains. -// -//nolint:gocritic // the signature is forced by kubebuilder -func (e *EnvoyFilterWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { - filter := &istionetworkingClientGo.EnvoyFilter{} - if err := e.Decoder.Decode(req, filter); err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - return e.createAdmissionResponse(ctx, filter) -} - -func (e *EnvoyFilterWebhook) createAdmissionResponse( - ctx context.Context, - filter *istionetworkingClientGo.EnvoyFilter, -) admission.Response { - // filter out envoyfilters that are not managed by this webhook - if !strings.HasPrefix(filter.Name, v1beta1constants.TechnicalIDPrefix) { - return admission.Allowed("requested object is not an EnvoyFilter managed by this webhook") - } - - aclExtension := &extensionsv1alpha1.Extension{} - err := e.Client.Get(ctx, types.NamespacedName{Name: ExtensionName, Namespace: filter.Name}, aclExtension) - - if client.IgnoreNotFound(err) != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - // if an error occured or the extension is in deletion, just allow without - // introducing any patches - if err != nil || !aclExtension.DeletionTimestamp.IsZero() { - return admission.Allowed(fmt.Sprintf("extension %s not enabled for shoot %s or is in deletion", ExtensionName, filter.Name)) - } - - extSpec := &extensionspec.ExtensionSpec{} - if err := json.Unmarshal(aclExtension.Spec.ProviderConfig.Raw, extSpec); err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - if err := controller.ValidateExtensionSpec(extSpec); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - cluster, err := helper.GetClusterForExtension(ctx, e.Client, aclExtension) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - var alwaysAllowedCIDRs []string - var shootSpecificCIRDs []string - - alwaysAllowedCIDRs = append(alwaysAllowedCIDRs, helper.GetSeedSpecificAllowedCIDRs(cluster.Seed)...) - - if len(e.AdditionalAllowedCIDRs) >= 1 { - alwaysAllowedCIDRs = append(alwaysAllowedCIDRs, e.AdditionalAllowedCIDRs...) - } - - // Gardener supports workerless Shoots. These don't have an associated - // Infrastructure object and don't need Node- or Pod-specific CIDRs to be - // allowed. Therefore, skip these steps for workerless Shoots. - if !v1beta1helper.IsWorkerless(cluster.Shoot) { - shootSpecificCIRDs = append(shootSpecificCIRDs, helper.GetShootNodeSpecificAllowedCIDRs(cluster.Shoot)...) - shootSpecificCIRDs = append(shootSpecificCIRDs, helper.GetShootPodSpecificAllowedCIDRs(cluster.Shoot)...) - - infra, err := helper.GetInfrastructureForExtension(ctx, e.Client, aclExtension, cluster.Shoot.Name) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - providerSpecificCIRDs, err := helper.GetProviderSpecificAllowedCIDRs(infra) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - shootSpecificCIRDs = append(shootSpecificCIRDs, providerSpecificCIRDs...) - } - - originalFilter := findTCPProxyFilter(filter) - if originalFilter == nil { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("filter with name \"envoy.filters.network.tcp_proxy\" not present in EnvoyFilter %s/%s", filter.Namespace, filter.Name)) - } - filterPatch, err := envoyfilters.CreateInternalFilterPatchFromRule(extSpec.Rule, alwaysAllowedCIDRs, shootSpecificCIRDs) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - filterPatchMap, err := filterPatch.AsMap() - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - // make sure the original filter is the last - filterPatches := []map[string]interface{}{filterPatchMap, originalFilter.AsMap()} - - return buildAdmissionResponseWithFilterPatches(filterPatches) -} - -func findTCPProxyFilter(filter *istionetworkingClientGo.EnvoyFilter) *structpb.Struct { - for _, configpatch := range filter.Spec.ConfigPatches { - patch := configpatch.Patch.Value - filters, ok := patch.Fields["filters"] - if !ok { - continue - } - filterList := filters.GetListValue() - if filterList == nil { - continue - } - for _, v := range filterList.GetValues() { - structVal := v.GetStructValue() - if structVal == nil { - continue - } - name, ok := structVal.Fields["name"] - if !ok { - continue - } - if name.GetStringValue() == "envoy.filters.network.tcp_proxy" { - return structVal - } - } - } - return nil -} - -func buildAdmissionResponseWithFilterPatches(filters []map[string]interface{}) admission.Response { - return admission.Response{ - Patches: []jsonpatch.Operation{ - { - Operation: "replace", - Path: "/spec/configPatches/0/patch/value/filters", - Value: filters, - }, - }, - AdmissionResponse: admissionv1.AdmissionResponse{ - Allowed: true, - PatchType: ptr.To(admissionv1.PatchTypeJSONPatch), - }, - } -} diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go deleted file mode 100644 index 2c7e223e8..000000000 --- a/pkg/webhook/webhook_test.go +++ /dev/null @@ -1,581 +0,0 @@ -package webhook - -import ( - "context" - "encoding/json" - "os" - "path" - "strings" - - openstackv1alpha1 "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1" - "github.com/gardener/gardener-extension-provider-openstack/pkg/openstack" - gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" - extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - istionetworkingClientGo "istio.io/client-go/pkg/apis/networking/v1alpha3" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/stackitcloud/gardener-extension-acl/pkg/envoyfilters" - "github.com/stackitcloud/gardener-extension-acl/pkg/extensionspec" -) - -var _ = Describe("webhook unit test", func() { - var ( - e *EnvoyFilterWebhook - ext *extensionsv1alpha1.Extension - cluster *extensionsv1alpha1.Cluster - infra *extensionsv1alpha1.Infrastructure - namespace string - name string - ) - - BeforeEach(func() { - name = "some-shoot" - namespace = createNewNamespace() - infra = getNewInfrastructure(namespace, name, "non-existent", []byte("{}"), []byte("{}")) - Expect(k8sClient.Create(ctx, infra)).To(Succeed()) - - // set up default shoot part of cluster resource - shoot := &gardencorev1beta1.Shoot{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Spec: gardencorev1beta1.ShootSpec{ - Networking: &gardencorev1beta1.Networking{ - Nodes: ptr.To("10.250.0.0/16"), - Pods: ptr.To("100.96.0.0/11"), - Services: ptr.To("100.64.0.0/13"), - Type: ptr.To("calico"), - }, - // we need a provider section with at least one worker pool, - // otherwise the Shoot will be considered workerless - Provider: gardencorev1beta1.Provider{ - Workers: []gardencorev1beta1.Worker{ - { - Name: "workerpool", - }, - }, - }, - }, - } - - // set up default seed part of cluster resource - seed := &gardencorev1beta1.Seed{ - Spec: gardencorev1beta1.SeedSpec{ - Networks: gardencorev1beta1.SeedNetworks{ - Nodes: ptr.To("100.250.0.0/16"), - Pods: "10.96.0.0/11", - Services: "10.64.0.0/13", - }, - }, - } - - cluster = getNewCluster(namespace, shoot, seed) - Expect(k8sClient.Create(ctx, cluster)).To(Succeed()) - - e = getNewWebhook() - }) - - AfterEach(func() { - deleteNamespace(namespace) - }) - - Describe("createAdmissionResponse", func() { - When("the name of the EnvoyFilter doesn't start with 'shoot-'", func() { - It("issues no patch for the EnvoyFilter", func() { - df := getEnvoyFilterFromFile("non-shoot-envoyfilter") - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - Expect(ar.Result.Message).To(ContainSubstring("not an EnvoyFilter managed by this webhook")) - Expect(ar.Patch).To(BeEmpty()) - }) - }) - - When("there is no extension", func() { - When("the shoot uses the legacy technical ID format 'shoot-'", func() { - It("issues no patch for the EnvoyFilter", func() { - df := getEnvoyFilterFromFile("shoot-foo-bar") - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - Expect(ar.Result.Message).To(ContainSubstring("not enabled for shoot")) - Expect(ar.Patch).To(BeEmpty()) - }) - }) - - When("the shoot uses the current technical ID format 'shoot--'", func() { - It("issues no patch for the EnvoyFilter", func() { - df := getEnvoyFilterFromFile(namespace) - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - Expect(ar.Result.Message).To(ContainSubstring("not enabled for shoot")) - Expect(ar.Patch).To(BeEmpty()) - }) - }) - }) - - When("there is an extension resource with one DENY rule", func() { - extSpec := getExtensionSpec() - - BeforeEach(func() { - addRuleToSpec(extSpec, "DENY", "source_ip", "0.0.0.0/0") - ext = getNewExtension(namespace, *extSpec) - - Expect(k8sClient.Create(ctx, ext)).To(Succeed()) - }) - - AfterEach(func() { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) - }) - - It("patches this rule into the filters object", func() { - df := getEnvoyFilterFromFile(namespace) - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - - expectedFilters := []map[string]interface{}{ - { - "name": "acl-internal-source_ip", - "typed_config": map[string]interface{}{ - "stat_prefix": "envoyrbac", - "rules": map[string]interface{}{ - "policies": map[string]interface{}{ - "acl-internal": map[string]interface{}{ - "permissions": buildInterfaceSlice([]map[string]interface{}{ - { - "any": true, - }, - }), - "principals": buildInterfaceSlice([]map[string]interface{}{ - { - "source_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": float64(0), - }, - }, - }), - }, - }, - "action": "DENY", - }, - "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", - }, - }, - - { - "name": "envoy.filters.network.tcp_proxy", - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", - "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - }, - }, - } - - Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) - }) - }) - - When("there is an extension resource with one ALLOW rule", func() { - extSpec := getExtensionSpec() - - BeforeEach(func() { - addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") - ext = getNewExtension(namespace, *extSpec) - - Expect(k8sClient.Create(ctx, ext)).To(Succeed()) - }) - - AfterEach(func() { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) - }) - - It("patches this rule into the filters object, including CIDRs for Seed|Shoot nodes and pods", func() { - df := getEnvoyFilterFromFile(namespace) - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - - expectedFilters := []map[string]interface{}{ - { - "name": "acl-internal-source_ip", - "typed_config": map[string]interface{}{ - "stat_prefix": "envoyrbac", - "rules": map[string]interface{}{ - "policies": map[string]interface{}{ - "acl-internal": map[string]interface{}{ - "permissions": buildInterfaceSlice([]map[string]interface{}{ - { - "any": true, - }, - }), - "principals": buildInterfaceSlice([]map[string]any{ - { - "source_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": float64(0), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "100.250.0.0", - "prefix_len": float64(16), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.96.0.0", - "prefix_len": float64(11), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.250.0.0", - "prefix_len": float64(16), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "100.96.0.0", - "prefix_len": float64(11), - }, - }, - }), - }, - }, - }, - "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", - }, - }, - - { - "name": "envoy.filters.network.tcp_proxy", - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", - "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - }, - }, - } - - Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) - }) - }) - - When("there is an extension resource with one ALLOW rule and infra is of type OpenStack", func() { - extSpec := getExtensionSpec() - - BeforeEach(func() { - addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") - ext = getNewExtension(namespace, *extSpec) - - Expect(k8sClient.Create(ctx, ext)).To(Succeed()) - - infra.Spec.Type = openstack.Type - Expect(k8sClient.Update(ctx, infra)).To(Succeed()) - - infraStatusJSON, err := json.Marshal(&openstackv1alpha1.InfrastructureStatus{ - TypeMeta: metav1.TypeMeta{ - Kind: "InfrastructureStatus", - APIVersion: "openstack.provider.extensions.gardener.cloud/v1alpha1", - }, - Networks: openstackv1alpha1.NetworkStatus{ - Router: openstackv1alpha1.RouterStatus{ - ID: "router-id", - IP: "10.9.8.7", - }, - }, - }) - Expect(err).To(BeNil()) - - infra.Status = extensionsv1alpha1.InfrastructureStatus{ - DefaultStatus: extensionsv1alpha1.DefaultStatus{ - ProviderStatus: &runtime.RawExtension{ - Raw: infraStatusJSON, - }, - }, - } - Expect(k8sClient.Status().Update(ctx, infra)).To(Succeed()) - }) - - AfterEach(func() { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) - }) - - It("patches this rule into the filters object, including CIDRs for Seed|Shoot nodes and pods and also the OpenStack router IP", func() { - df := getEnvoyFilterFromFile(namespace) - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - - expectedFilters := []map[string]interface{}{ - { - "name": "acl-internal-source_ip", - "typed_config": map[string]interface{}{ - "stat_prefix": "envoyrbac", - "rules": map[string]interface{}{ - "policies": map[string]interface{}{ - "acl-internal": map[string]interface{}{ - "permissions": buildInterfaceSlice([]map[string]interface{}{ - { - "any": true, - }, - }), - "principals": buildInterfaceSlice([]map[string]any{ - { - "source_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": float64(0), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "100.250.0.0", - "prefix_len": float64(16), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.96.0.0", - "prefix_len": float64(11), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.250.0.0", - "prefix_len": float64(16), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "100.96.0.0", - "prefix_len": float64(11), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.9.8.7", - "prefix_len": float64(32), - }, - }, - }), - }, - }, - }, - "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", - }, - }, - - { - "name": "envoy.filters.network.tcp_proxy", - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", - "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - }, - }, - } - - Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) - }) - }) - - When("the Shoot is workerless, and there is one allow rule", func() { - extSpec := getExtensionSpec() - - BeforeEach(func() { - addRuleToSpec(extSpec, "ALLOW", "source_ip", "0.0.0.0/0") - ext = getNewExtension(namespace, *extSpec) - Expect(k8sClient.Create(ctx, ext)).To(Succeed()) - - // define a workerless shoot - shoot := &gardencorev1beta1.Shoot{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Spec: gardencorev1beta1.ShootSpec{}, - } - shootJSON, err := json.Marshal(shoot) - Expect(err).To(BeNil()) - - cluster.Spec.Shoot = runtime.RawExtension{Raw: shootJSON} - Expect(k8sClient.Update(ctx, cluster)).To(Succeed()) - }) - - AfterEach(func() { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ext))).To(Succeed()) - }) - - It("patches only the rule into the filters object, and no CIDRs for Seed|Shoot nodes and pods", func() { - df := getEnvoyFilterFromFile(namespace) - - ar := e.createAdmissionResponse(context.Background(), df) - - Expect(ar.Allowed).To(BeTrue()) - - expectedFilters := []map[string]interface{}{ - { - "name": "acl-internal-source_ip", - "typed_config": map[string]interface{}{ - "stat_prefix": "envoyrbac", - "rules": map[string]interface{}{ - "policies": map[string]interface{}{ - "acl-internal": map[string]interface{}{ - "permissions": buildInterfaceSlice([]map[string]interface{}{ - { - "any": true, - }, - }), - "principals": buildInterfaceSlice([]map[string]any{ - { - "source_ip": map[string]interface{}{ - "address_prefix": "0.0.0.0", - "prefix_len": float64(0), - }, - }, - { - - "remote_ip": map[string]interface{}{ - "address_prefix": "100.250.0.0", - "prefix_len": float64(16), - }, - }, - { - "remote_ip": map[string]interface{}{ - "address_prefix": "10.96.0.0", - "prefix_len": float64(11), - }, - }, - }), - }, - }, - }, - "@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC", - }, - }, - - { - "name": "envoy.filters.network.tcp_proxy", - "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", - "cluster": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - "stat_prefix": "outbound|443||kube-apiserver." + namespace + ".svc.cluster.local", - }, - }, - } - - Expect(ar.Patches[0].Value).To(Equal(expectedFilters)) - }) - }) - }) -}) - -func getNewWebhook() *EnvoyFilterWebhook { - decoder := admission.NewDecoder(clientScheme) - return &EnvoyFilterWebhook{ - Client: k8sClient, - Decoder: decoder, - } -} - -func getNewCluster(namespace string, shoot *gardencorev1beta1.Shoot, seed *gardencorev1beta1.Seed) *extensionsv1alpha1.Cluster { - shootJSON, err := json.Marshal(shoot) - Expect(err).To(BeNil()) - - seedJSON, err := json.Marshal(seed) - Expect(err).To(BeNil()) - - return &extensionsv1alpha1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespace, - }, - Spec: extensionsv1alpha1.ClusterSpec{ - CloudProfile: runtime.RawExtension{Raw: []byte("{}")}, - Seed: runtime.RawExtension{Raw: seedJSON}, - Shoot: runtime.RawExtension{Raw: shootJSON}, - }, - } -} - -func getNewInfrastructure( - namespace, name, typeName string, - providerConfigJSON, providerStatusJSON []byte, -) *extensionsv1alpha1.Infrastructure { - return &extensionsv1alpha1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: extensionsv1alpha1.InfrastructureSpec{ - DefaultSpec: extensionsv1alpha1.DefaultSpec{ - Type: typeName, - ProviderConfig: &runtime.RawExtension{ - Raw: providerConfigJSON, - }, - }, - }, - Status: extensionsv1alpha1.InfrastructureStatus{ - DefaultStatus: extensionsv1alpha1.DefaultStatus{ - ProviderStatus: &runtime.RawExtension{ - Raw: providerStatusJSON, - }, - }, - }, - } -} - -func getExtensionSpec() *extensionspec.ExtensionSpec { - return &extensionspec.ExtensionSpec{ - Rule: &envoyfilters.ACLRule{}, - } -} - -func addRuleToSpec(extSpec *extensionspec.ExtensionSpec, action, ruleType, cidr string) { - extSpec.Rule = &envoyfilters.ACLRule{ - Cidrs: []string{ - cidr, - }, - Action: action, - Type: ruleType, - } -} - -// getEnvoyFilterFromFile takes the technical shoot ID as a parameter to render -// into the JSON tempate file. Returns both the JSON representation as string -// and the struct type. -func getEnvoyFilterFromFile(technicalID string) (filter *istionetworkingClientGo.EnvoyFilter) { - filterSpecJSON, err := os.ReadFile(path.Join("./testdata", "defaultEnvoyFilter.json")) - Expect(err).ShouldNot(HaveOccurred()) - - templatedFilterSpec := strings.ReplaceAll(string(filterSpecJSON), "{{TECHNICAL-SHOOT-ID}}", technicalID) - - filter = &istionetworkingClientGo.EnvoyFilter{} - - Expect(json.Unmarshal([]byte(templatedFilterSpec), filter)).To(Succeed()) - - return filter -} - -// buildInterfaceSlice can be used to create a slice of interface which is not possible when using literals. -func buildInterfaceSlice(maps []map[string]any) (ret []any) { - for _, m := range maps { - ret = append(ret, m) - } - return -}