Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[maistra-2.6] Rebase commits between 2.5.0 and 2.5.2 #1016

Merged
merged 7 commits into from
Jun 12, 2024
11 changes: 1 addition & 10 deletions pilot/pkg/bootstrap/configcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}).
Expand Down
5 changes: 4 additions & 1 deletion pilot/pkg/config/kube/crdclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions pilot/pkg/config/kube/gateway/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
Expand Down Expand Up @@ -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),
})
}
Expand Down
3 changes: 1 addition & 2 deletions pilot/pkg/config/kube/gateway/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/config/kube/gateway/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import (

var (
gatewayClassSpec = &k8s.GatewayClassSpec{
ControllerName: constants.ManagedGatewayController,
ControllerName: controllerName,
}
gatewaySpec = &k8s.GatewaySpec{
GatewayClassName: "gwclass",
Expand Down
28 changes: 2 additions & 26 deletions pilot/pkg/config/kube/gateway/deploymentcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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...)
Expand Down Expand Up @@ -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)
}
Expand Down
15 changes: 6 additions & 9 deletions pilot/pkg/config/kube/gateway/deploymentcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -62,7 +61,7 @@ func TestConfigureIstioGateway(t *testing.T) {
Name: "custom",
},
Spec: v1beta1.GatewayClassSpec{
ControllerName: constants.ManagedGatewayController,
ControllerName: controllerName,
},
}
defaultObjects := []runtime.Object{defaultNamespace}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
Expand All @@ -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))
Expand Down
23 changes: 20 additions & 3 deletions pkg/config/analysis/incluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -50,21 +52,26 @@ 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,
DomainSuffix: domainSuffix,
Identifier: "analysis-controller",
FiltersByGVK: ia.GetFiltersByGVK(),
},
all.Remove(rwConfigStore.Schemas().All()...))

schemas)
ia.AddSource(store)
kubeClient.RunAndWait(stop)
err := ia.Init(stop)
Expand All @@ -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]{}
Expand Down
1 change: 0 additions & 1 deletion pkg/config/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions pkg/revisions/tag_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion pkg/servicemesh/federation/discovery/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{}),
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions pkg/servicemesh/federation/federation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions releasenotes/notes/custom-gw-classname.yaml
Original file line number Diff line number Diff line change
@@ -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`.
15 changes: 12 additions & 3 deletions samples/extauthz/cmd/extauthz/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
},
},
{
Expand Down Expand Up @@ -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()),
},
},
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 "<too-long>"
}
return body
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
}
Expand Down
Loading