diff --git a/pilot/pkg/bootstrap/configcontroller.go b/pilot/pkg/bootstrap/configcontroller.go index 9697a604b38..d40a496bd79 100644 --- a/pilot/pkg/bootstrap/configcontroller.go +++ b/pilot/pkg/bootstrap/configcontroller.go @@ -39,7 +39,6 @@ import ( "istio.io/istio/pkg/config/schema/collections" "istio.io/istio/pkg/config/schema/gvr" "istio.io/istio/pkg/log" - "istio.io/istio/pkg/revisions" ) // URL schemes supported by the config store @@ -182,22 +181,14 @@ func (s *Server) initK8SConfigStore(args *PilotArgs) error { AddRunFunction(func(leaderStop <-chan struct{}) { // We can only run this if the Gateway CRD is created if s.kubeClient.CrdWatcher().WaitForCRD(gvr.KubernetesGateway, leaderStop) { - var tagWatcher revisions.TagWatcher - // TagWatcher requires permission for MutatingWebhook, so it can't be used in multi-tenant mode - if !s.kubeClient.IsMultiTenant() { - tagWatcher = revisions.NewTagWatcher(s.kubeClient, args.Revision) - } controller := gateway.NewDeploymentController(s.kubeClient, s.clusterID, s.environment, - s.webhookInfo.getWebhookConfig, s.webhookInfo.addHandler, tagWatcher, args.Revision) + s.webhookInfo.getWebhookConfig, s.webhookInfo.addHandler, args.Revision) // Start informers again. This fixes the case where informers for namespace do not start, // as we create them only after acquiring the leader lock // Note: stop here should be the overall pilot stop, NOT the leader election stop. We are // basically lazy loading the informer, if we stop it when we lose the lock we will never // recreate it again. s.kubeClient.RunAndWait(stop) - if tagWatcher != nil { - go tagWatcher.Run(leaderStop) - } controller.Run(leaderStop) } }). diff --git a/pilot/pkg/config/kube/crdclient/client.go b/pilot/pkg/config/kube/crdclient/client.go index 266a395b0b8..eef8fca2298 100644 --- a/pilot/pkg/config/kube/crdclient/client.go +++ b/pilot/pkg/config/kube/crdclient/client.go @@ -339,7 +339,10 @@ func (cl *Client) addCRD(name string) { resourceGVK := s.GroupVersionKind() gvr := s.GroupVersionResource() - if cl.client.IsMultiTenant() && resourceGVK == gvk.GatewayClass { + if !features.EnableGatewayAPI && s.Group() == gvk.KubernetesGateway.Group { + scope.Infof("Skipping CRD %v as GatewayAPI support is not enabled", s.GroupVersionKind()) + return + } else if cl.client.IsMultiTenant() && resourceGVK == gvk.GatewayClass { scope.Infof("Skipping CRD %v as it is not compatible with maistra multi-tenancy", s.GroupVersionKind()) return } diff --git a/pilot/pkg/config/kube/gateway/conditions.go b/pilot/pkg/config/kube/gateway/conditions.go index 408c6c340b6..35e7e781392 100644 --- a/pilot/pkg/config/kube/gateway/conditions.go +++ b/pilot/pkg/config/kube/gateway/conditions.go @@ -24,7 +24,6 @@ import ( "istio.io/istio/pilot/pkg/model/kstatus" "istio.io/istio/pkg/config" - "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/schema/gvk" "istio.io/istio/pkg/maps" "istio.io/istio/pkg/ptr" @@ -48,7 +47,7 @@ func createRouteStatus(parentResults []RouteParentResult, obj config.Config, cur // gateway controllers that are exposing their status on the same route. We need to attempt to manage ours properly (including // removing gateway references when they are removed), without mangling other Controller's status. for _, r := range currentParents { - if r.ControllerName != constants.ManagedGatewayController { + if r.ControllerName != controllerName { // We don't own this status, so keep it around parents = append(parents, r) } @@ -160,7 +159,7 @@ func createRouteStatus(parentResults []RouteParentResult, obj config.Config, cur } parents = append(parents, k8s.RouteParentStatus{ ParentRef: gw.OriginalReference, - ControllerName: constants.ManagedGatewayController, + ControllerName: controllerName, Conditions: setConditions(obj.Generation, currentConditions, conds), }) } diff --git a/pilot/pkg/config/kube/gateway/conditions_test.go b/pilot/pkg/config/kube/gateway/conditions_test.go index 52a0dce0e3f..4276c0336fa 100644 --- a/pilot/pkg/config/kube/gateway/conditions_test.go +++ b/pilot/pkg/config/kube/gateway/conditions_test.go @@ -22,7 +22,6 @@ import ( k8s "sigs.k8s.io/gateway-api/apis/v1beta1" "istio.io/istio/pkg/config" - "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/schema/gvk" ) @@ -32,7 +31,7 @@ func TestCreateRouteStatus(t *testing.T) { parentStatus := []k8s.RouteParentStatus{ { ParentRef: parentRef, - ControllerName: constants.ManagedGatewayController, + ControllerName: controllerName, Conditions: []metav1.Condition{ { Type: string(k8s.RouteReasonAccepted), diff --git a/pilot/pkg/config/kube/gateway/controller_test.go b/pilot/pkg/config/kube/gateway/controller_test.go index 9b6d8242390..00e2039edce 100644 --- a/pilot/pkg/config/kube/gateway/controller_test.go +++ b/pilot/pkg/config/kube/gateway/controller_test.go @@ -43,7 +43,7 @@ import ( var ( gatewayClassSpec = &k8s.GatewayClassSpec{ - ControllerName: constants.ManagedGatewayController, + ControllerName: controllerName, } gatewaySpec = &k8s.GatewaySpec{ GatewayClassName: "gwclass", diff --git a/pilot/pkg/config/kube/gateway/deploymentcontroller.go b/pilot/pkg/config/kube/gateway/deploymentcontroller.go index 418e94ab562..79346235334 100644 --- a/pilot/pkg/config/kube/gateway/deploymentcontroller.go +++ b/pilot/pkg/config/kube/gateway/deploymentcontroller.go @@ -32,7 +32,6 @@ import ( gateway "sigs.k8s.io/gateway-api/apis/v1beta1" "sigs.k8s.io/yaml" - "istio.io/api/label" meshapi "istio.io/api/mesh/v1alpha1" "istio.io/istio/pilot/pkg/features" "istio.io/istio/pilot/pkg/model" @@ -46,7 +45,6 @@ import ( "istio.io/istio/pkg/kube/inject" "istio.io/istio/pkg/kube/kclient" istiolog "istio.io/istio/pkg/log" - "istio.io/istio/pkg/revisions" "istio.io/istio/pkg/test/util/tmpl" "istio.io/istio/pkg/test/util/yml" "istio.io/istio/pkg/util/sets" @@ -90,7 +88,6 @@ type DeploymentController struct { services kclient.Client[*corev1.Service] serviceAccounts kclient.Client[*corev1.ServiceAccount] namespaces kclient.Client[*corev1.Namespace] - tagWatcher revisions.TagWatcher revision string defaultLabels map[string]string } @@ -165,7 +162,7 @@ func getClassInfos() map[gateway.GatewayController]classInfo { // NewDeploymentController constructs a DeploymentController and registers required informers. // The controller will not start until Run() is called. func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *model.Environment, - webhookConfig func() inject.WebhookConfig, injectionHandler func(fn func()), tw revisions.TagWatcher, revision string, + webhookConfig func() inject.WebhookConfig, injectionHandler func(fn func()), revision string, ) *DeploymentController { dc := &DeploymentController{ client: client, @@ -226,8 +223,6 @@ func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *mode } } })) - dc.tagWatcher = tw - dc.tagWatcher.AddHandler(dc.HandleTagChange) } // On injection template change, requeue all gateways @@ -252,7 +247,7 @@ func (d *DeploymentController) Run(stop <-chan struct{}) { syncFuncs := []cache.InformerSynced{d.deployments.HasSynced, d.services.HasSynced, d.serviceAccounts.HasSynced, d.gateways.HasSynced} shutdownFuncs := []controllers.Shutdowner{d.deployments, d.services, d.serviceAccounts, d.gateways} if !d.client.IsMultiTenant() { - syncFuncs = append(syncFuncs, d.namespaces.HasSynced, d.gatewayClasses.HasSynced, d.tagWatcher.HasSynced) + syncFuncs = append(syncFuncs, d.namespaces.HasSynced, d.gatewayClasses.HasSynced) shutdownFuncs = append(shutdownFuncs, d.namespaces, d.gatewayClasses) } kube.WaitForCacheSync("deployment controller", stop, syncFuncs...) @@ -291,25 +286,6 @@ func (d *DeploymentController) Reconcile(req types.NamespacedName) error { return nil } - if d.namespaces != nil { - // find the tag or revision indicated by the object - selectedTag, ok := gw.Labels[label.IoIstioRev.Name] - if !ok { - ns := d.namespaces.Get(gw.Namespace, "") - if ns == nil { - log.Debugf("gateway is not for this revision, skipping") - return nil - } - selectedTag = ns.Labels[label.IoIstioRev.Name] - } - myTags := d.tagWatcher.GetMyTags() - if !myTags.Contains(selectedTag) && !(selectedTag == "" && myTags.Contains("default")) { - log.Debugf("gateway is not for this revision, skipping") - return nil - } - } - // TODO: Here we could check if the tag is set and matches no known tags, and handle that if we are default. - // Matched class, reconcile it return d.configureIstioGateway(log, *gw, ci) } diff --git a/pilot/pkg/config/kube/gateway/deploymentcontroller_test.go b/pilot/pkg/config/kube/gateway/deploymentcontroller_test.go index d4cab9f7ed6..f1865ea982b 100644 --- a/pilot/pkg/config/kube/gateway/deploymentcontroller_test.go +++ b/pilot/pkg/config/kube/gateway/deploymentcontroller_test.go @@ -47,7 +47,6 @@ import ( "istio.io/istio/pkg/kube/kclient" "istio.io/istio/pkg/kube/kclient/clienttest" istiolog "istio.io/istio/pkg/log" - "istio.io/istio/pkg/revisions" "istio.io/istio/pkg/test" "istio.io/istio/pkg/test/env" "istio.io/istio/pkg/test/util/assert" @@ -62,7 +61,7 @@ func TestConfigureIstioGateway(t *testing.T) { Name: "custom", }, Spec: v1beta1.GatewayClassSpec{ - ControllerName: constants.ManagedGatewayController, + ControllerName: controllerName, }, } defaultObjects := []runtime.Object{defaultNamespace} @@ -238,11 +237,9 @@ func TestConfigureIstioGateway(t *testing.T) { stop := test.NewStop(t) env := model.NewEnvironment() env.PushContext().ProxyConfigs = tt.pcs - tw := revisions.NewTagWatcher(client, "") - go tw.Run(stop) d := NewDeploymentController( client, cluster.ID(features.ClusterName), env, testInjectionConfig(t), func(fn func()) { - }, tw, "") + }, "") d.patcher = func(gvr schema.GroupVersionResource, name string, namespace string, data []byte, subresources ...string) error { b, err := yaml.JSONToYAML(data) if err != nil { @@ -270,9 +267,8 @@ func TestVersionManagement(t *testing.T) { Name: "default", }, }) - tw := revisions.NewTagWatcher(c, "default") env := &model.Environment{} - d := NewDeploymentController(c, "", env, testInjectionConfig(t), func(fn func()) {}, tw, "") + d := NewDeploymentController(c, "", env, testInjectionConfig(t), func(fn func()) {}, "") reconciles := atomic.NewInt32(0) wantReconcile := int32(0) expectReconciled := func() { @@ -296,7 +292,6 @@ func TestVersionManagement(t *testing.T) { } stop := test.NewStop(t) gws := clienttest.Wrap(t, d.gateways) - go tw.Run(stop) go d.Run(stop) c.RunAndWait(stop) kube.WaitForCacheSync("test", stop, d.queue.HasSynced) @@ -306,7 +301,9 @@ func TestVersionManagement(t *testing.T) { Name: "gw", Namespace: "default", }, - Spec: v1beta1.GatewaySpec{GatewayClassName: defaultClassName}, + Spec: v1beta1.GatewaySpec{ + GatewayClassName: defaultClassName, + }, } gws.Create(defaultGateway) assert.Equal(t, assert.ChannelHasItem(t, writes), buildPatch(ControllerVersion)) diff --git a/pkg/config/analysis/incluster/controller.go b/pkg/config/analysis/incluster/controller.go index 8d70445819b..f6b92514890 100644 --- a/pkg/config/analysis/incluster/controller.go +++ b/pkg/config/analysis/incluster/controller.go @@ -33,6 +33,8 @@ import ( "istio.io/istio/pkg/config/analysis/local" "istio.io/istio/pkg/config/legacy/util/kuberesource" "istio.io/istio/pkg/config/resource" + "istio.io/istio/pkg/config/schema/collection" + "istio.io/istio/pkg/config/schema/collections" "istio.io/istio/pkg/kube" "istio.io/istio/pkg/log" "istio.io/istio/pkg/util/concurrent" @@ -50,12 +52,18 @@ func NewController(stop <-chan struct{}, rwConfigStore model.ConfigStoreControll kubeClient kube.Client, revision, namespace string, statusManager *status.Manager, domainSuffix string, ) (*Controller, error) { analyzer := analyzers.AllCombined() - all := kuberesource.ConvertInputsToSchemas(analyzer.Metadata().Inputs) ia := local.NewIstiodAnalyzer(analyzer, "", resource.Namespace(namespace), func(name config.GroupVersionKind) {}) ia.AddSource(rwConfigStore) + schemas := kuberesource.ConvertInputsToSchemas(analyzer.Metadata().Inputs). + Remove(collections.MeshConfig). // this is not an actual resource + Remove(collections.MeshNetworks) // this is not an actual resource + if kubeClient.IsMultiTenant() { + schemas = removeClusterScoped(schemas) + } // Filter out configs watched by rwConfigStore so we don't watch multiple times + schemas = schemas.Remove(rwConfigStore.Schemas().All()...) store := crdclient.NewForSchemas(kubeClient, crdclient.Option{ Revision: revision, @@ -63,8 +71,7 @@ func NewController(stop <-chan struct{}, rwConfigStore model.ConfigStoreControll Identifier: "analysis-controller", FiltersByGVK: ia.GetFiltersByGVK(), }, - all.Remove(rwConfigStore.Schemas().All()...)) - + schemas) ia.AddSource(store) kubeClient.RunAndWait(stop) err := ia.Init(stop) @@ -83,6 +90,16 @@ func NewController(stop <-chan struct{}, rwConfigStore model.ConfigStoreControll return &Controller{analyzer: ia, statusctl: ctl}, nil } +func removeClusterScoped(schemas collection.Schemas) collection.Schemas { + b := collection.NewSchemasBuilder() + for _, s := range schemas.All() { + if !s.IsClusterScoped() { + b.MustAdd(s) + } + } + return b.Build() +} + // Run is blocking func (c *Controller) Run(stop <-chan struct{}) { db := concurrent.Debouncer[config.GroupVersionKind]{} diff --git a/pkg/config/constants/constants.go b/pkg/config/constants/constants.go index e38a87c6b01..e20e2e4d7dc 100644 --- a/pkg/config/constants/constants.go +++ b/pkg/config/constants/constants.go @@ -160,7 +160,6 @@ const ( WaypointServiceAccount = "istio.io/for-service-account" ManagedGatewayLabel = "gateway.istio.io/managed" - ManagedGatewayController = "istio.io/gateway-controller" UnmanagedGatewayController = "istio.io/unmanaged-gateway" ManagedGatewayControllerLabel = "istio.io-gateway-controller" ManagedGatewayMeshControllerLabel = "istio.io-mesh-controller" diff --git a/pkg/revisions/tag_watcher.go b/pkg/revisions/tag_watcher.go index 76182628a4e..9989ae173e3 100644 --- a/pkg/revisions/tag_watcher.go +++ b/pkg/revisions/tag_watcher.go @@ -28,6 +28,9 @@ import ( // TagWatcher keeps track of the current tags and can notify watchers // when the tags change. +// +// TagWatcher is not supported, because it watches MutatingWebhooks and relies on istio.io/rev label, +// which can be duplicated in OSSM, so it cannot work until we use revisions in our implementation of multi-tenancy. type TagWatcher interface { Run(stopCh <-chan struct{}) HasSynced() bool diff --git a/pkg/servicemesh/federation/discovery/controller.go b/pkg/servicemesh/federation/discovery/controller.go index 88e96ecc58e..30b84406496 100644 --- a/pkg/servicemesh/federation/discovery/controller.go +++ b/pkg/servicemesh/federation/discovery/controller.go @@ -60,6 +60,7 @@ type Controller struct { localNetwork string localClusterID string rm common.ResourceManager + resyncPeriod time.Duration env *model.Environment federationManager server.FederationManager statusManager status.Manager @@ -88,6 +89,7 @@ func NewController(opt Options) (*Controller, error) { localClusterID: opt.LocalClusterID, localNetwork: opt.LocalNetwork, rm: opt.ResourceManager, + resyncPeriod: opt.ResyncPeriod, env: opt.Env, sc: opt.ServiceController, stopChannels: make(map[cluster.ID]chan struct{}), @@ -212,7 +214,7 @@ func (c *Controller) update(ctx context.Context, instance *v1.ServiceMeshPeer) e ConfigStore: c.ConfigStoreController, StatusHandler: statusHandler, XDSUpdater: c.xds, - ResyncPeriod: time.Minute * 5, + ResyncPeriod: c.resyncPeriod, DomainSuffix: c.env.DomainSuffix, LocalClusterID: c.localClusterID, LocalNetwork: c.localNetwork, diff --git a/pkg/servicemesh/federation/federation.go b/pkg/servicemesh/federation/federation.go index ffcc7a6cd54..e88c56da01d 100644 --- a/pkg/servicemesh/federation/federation.go +++ b/pkg/servicemesh/federation/federation.go @@ -134,6 +134,7 @@ func internalNew(opt Options, cs maistraclient.Interface) (*Federation, error) { } discoveryController, err := discovery.NewController(discovery.Options{ ResourceManager: resourceManager, + ResyncPeriod: opt.ResyncPeriod, LocalClusterID: opt.LocalClusterID, LocalNetwork: opt.LocalNetwork, ServiceController: opt.ServiceController, diff --git a/releasenotes/notes/custom-gw-classname.yaml b/releasenotes/notes/custom-gw-classname.yaml new file mode 100644 index 00000000000..3af715a7867 --- /dev/null +++ b/releasenotes/notes/custom-gw-classname.yaml @@ -0,0 +1,7 @@ +apiVersion: release-notes/v2 +kind: feature +area: traffic-management +releaseNotes: +- | + **Added** an environment variable for istiod `PILOT_GATEWAY_API_DEFAULT_GATEWAYCLASS_NAME` that allows overriding the name of the default `GatewayClass` Gateway API resource. The default value is `istio`. + **Added** an environment variable for istiod `PILOT_GATEWAY_API_CONTROLLER_NAME` that allows overriding the name of the Istio Gateway API controller as exposed in the `spec.controllerName` field in the `GatewayClass` resource. The default value is `istio.io/gateway-controller`. diff --git a/samples/extauthz/cmd/extauthz/main.go b/samples/extauthz/cmd/extauthz/main.go index 2b32f1ce11f..70c33f616e6 100644 --- a/samples/extauthz/cmd/extauthz/main.go +++ b/samples/extauthz/cmd/extauthz/main.go @@ -187,7 +187,7 @@ func (s *extAuthzServerV3) allow(request *authv3.CheckRequest) *authv3.CheckResp { Header: &corev3.HeaderValue{ Key: receivedHeader, - Value: request.GetAttributes().String(), + Value: returnIfNotTooLong(request.GetAttributes().String()), }, }, { @@ -220,7 +220,7 @@ func (s *extAuthzServerV3) deny(request *authv3.CheckRequest) *authv3.CheckRespo { Header: &corev3.HeaderValue{ Key: receivedHeader, - Value: request.GetAttributes().String(), + Value: returnIfNotTooLong(request.GetAttributes().String()), }, }, { @@ -262,7 +262,7 @@ func (s *ExtAuthzServer) ServeHTTP(response http.ResponseWriter, request *http.R if err != nil { log.Printf("[HTTP] read body failed: %v", err) } - l := fmt.Sprintf("%s %s%s, headers: %v, body: [%s]\n", request.Method, request.Host, request.URL, request.Header, body) + l := fmt.Sprintf("%s %s%s, headers: %v, body: [%s]\n", request.Method, request.Host, request.URL, request.Header, returnIfNotTooLong(string(body))) if allowedValue == request.Header.Get(checkHeader) { log.Printf("[HTTP][allowed]: %s", l) response.Header().Set(resultHeader, resultAllowed) @@ -358,3 +358,12 @@ func main() { signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) <-sigs } + +func returnIfNotTooLong(body string) string { + // Maximum size of a header accepted by Envoy is 60KiB, so when the request body is bigger than 60KB, + // we don't return it in a response header to avoid rejecting it by Envoy and returning 431 to the client + if len(body) > 60000 { + return "" + } + return body +} diff --git a/tests/integration/servicemesh/federation/discovery/discovery_test.go b/tests/integration/servicemesh/federation/discovery/discovery_test.go index 9f7b3c77f89..af9c3b5e2e2 100644 --- a/tests/integration/servicemesh/federation/discovery/discovery_test.go +++ b/tests/integration/servicemesh/federation/discovery/discovery_test.go @@ -55,10 +55,9 @@ var ( ServicePort: 7070, } - // Timeout is 6 minutes long, because we have hardcoded resync period 5 minutes long in the federation discovery controller. defaultRetry = echo.Retry{ Options: []retry.Option{ - retry.Timeout(6 * time.Minute), + retry.Timeout(30 * time.Second), retry.Delay(1 * time.Second), }, } diff --git a/tests/integration/servicemesh/federation/federation.go b/tests/integration/servicemesh/federation/federation.go index f6573bc0a48..c2d58035111 100644 --- a/tests/integration/servicemesh/federation/federation.go +++ b/tests/integration/servicemesh/federation/federation.go @@ -48,6 +48,15 @@ func SetupConfig(_ resource.Context, cfg *istio.Config) { cfg.DifferentTrustDomains = true cfg.ControlPlaneValues = ` components: + pilot: + k8s: + overlays: + - apiVersion: apps/v1 + kind: Deployment + name: istiod + patches: + - path: spec.template.spec.containers.[name:discovery].args[-1] + value: "--resync=3s" ingressGateways: - name: federation-ingress namespace: istio-system diff --git a/tests/integration/servicemesh/federation/ha/ha_test.go b/tests/integration/servicemesh/federation/ha/ha_test.go index 46e03afe14b..1bbddf3f4bc 100644 --- a/tests/integration/servicemesh/federation/ha/ha_test.go +++ b/tests/integration/servicemesh/federation/ha/ha_test.go @@ -52,10 +52,9 @@ var ( ServicePort: 7070, } - // Timeout is 6 minutes long, because we have hardcoded resync period 5 minutes long in the federation discovery controller. defaultRetry = echo.Retry{ Options: []retry.Option{ - retry.Timeout(6 * time.Minute), + retry.Timeout(30 * time.Second), retry.Delay(1 * time.Second), }, } @@ -119,7 +118,7 @@ spec: aSecondary.CallOrFail(t, withDefaults(echo.CallOptions{ Address: fmt.Sprintf("b.%s.svc.cluster.local", ns.Name()), - Count: 2, + Count: 5, Check: check.And( check.GRPCStatus(codes.OK), check.ReachedClusters(t.AllClusters(), []cluster.Cluster{primary, secondary}),