Skip to content

Commit

Permalink
[no-cluster-rbac] OSSM-2255: Preliminary support for Gateway API (mai…
Browse files Browse the repository at this point in the history
…stra#679)

* [no-cluster-rbac] OSSM-1423: Preliminary support for Gateway API

This adds support for Gateway API in a multi-tenant OSSM install, plus
integration tests. It also improves upon our current test suite a bit.

Caveats:
- we don't support namespace selectors in the AllowedRoutes field
- GatewayClasses are ignored (but specifying "istio" always works)

* OSSM-2123 gw-api deploymentcontroller should not set securityContext (maistra#649)

I'm removing the entire block here because it clashes with deploying to
a project in OCP that uses the 'restricted' SCC (which is the default).
If people need different securityContext settings, they can still set
them in the gateway injection template.

* chore: uses arg position in string formatting

this way we can reuse the GatewayAPI flag without passing it multiple times.

* chore: aligns pkg alias with its actual version

* fix: updates gateway CRD with v1beta1 definition

* fix(rbac): adds missing resources

* fix(tests): restores discovery and routing roles

* Revert "fix: updates gateway CRD with v1beta1 definition"

This reverts commit 6d48649.

* Revert "fix(rbac): adds missing resources"

This reverts commit 07163f3.

* Revert "fix(tests): restores discovery and routing roles"

This reverts commit a1f109a.

* fix(tests): aligns test changes with maistra-2.3

Based on 8f27a9e

* fix: do not set nsInformer/Lister if mrc in use

* fix: removed obsolote API version for 1.16

* fix: changed kind: Mesh to kind: Service for v1beta1

- moves gw YAMLs to their own funcs

* chore: moves apply funcs around

* fix: adds ReferenceGrant

This is required in order to have access to ns cross-tls secret from the gateway ns

Co-authored-by: Daniel Grimm <dgrimm@redhat.com>

Don't use tag watcher in multi-tenant mode

Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>

Align Gateway API tests with pilot/gateway_test

Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
  • Loading branch information
bartoszmajsak authored and openshift-merge-robot committed Aug 10, 2023
1 parent 85417b7 commit e466ba0
Show file tree
Hide file tree
Showing 40 changed files with 599 additions and 674 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,30 +55,6 @@ spec:
- name: istio-proxy
image: "{{ .ProxyImage }}"
{{with .Values.global.imagePullPolicy }}imagePullPolicy: "{{.}}"{{end}}
securityContext:
{{- if .KubeVersion122 }}
# Safe since 1.22: https://github.com/kubernetes/kubernetes/pull/103326
capabilities:
drop:
- ALL
allowPrivilegeEscalation: false
privileged: false
readOnlyRootFilesystem: true
runAsUser: 1337
runAsGroup: 1337
runAsNonRoot: true
{{- else }}
capabilities:
drop:
- ALL
add:
- NET_BIND_SERVICE
runAsUser: 0
runAsGroup: 1337
runAsNonRoot: false
allowPrivilegeEscalation: true
readOnlyRootFilesystem: true
{{- end }}
ports:
- containerPort: 15021
name: status-port
Expand Down
10 changes: 8 additions & 2 deletions pilot/pkg/bootstrap/configcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ func (s *Server) initK8SConfigStore(args *PilotArgs) error {
AddRunFunction(func(leaderStop <-chan struct{}) {
// We can only run this if the Gateway CRD is created
if configController.WaitForCRD(gvk.KubernetesGateway, leaderStop) {
tagWatcher := revisions.NewTagWatcher(s.kubeClient, args.Revision)
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)
// Start informers again. This fixes the case where informers for namespace do not start,
Expand All @@ -184,7 +188,9 @@ func (s *Server) initK8SConfigStore(args *PilotArgs) error {
// basically lazy loading the informer, if we stop it when we lose the lock we will never
// recreate it again.
s.kubeClient.RunAndWait(stop)
go tagWatcher.Run(leaderStop)
if tagWatcher != nil {
go tagWatcher.Run(leaderStop)
}
controller.Run(leaderStop)
}
}).
Expand Down
5 changes: 5 additions & 0 deletions pilot/pkg/config/kube/crdclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@ func handleCRDAdd(cl *Client, name string) {
resourceGVK := s.GroupVersionKind()
gvr := s.GroupVersionResource()

if cl.client.IsMultiTenant() && resourceGVK == gvk.GatewayClass {
scope.Infof("Skipping CRD %v as it is not compatible with maistra multi-tenancy", s.GroupVersionKind())
return
}

cl.kindsMu.Lock()
defer cl.kindsMu.Unlock()
if _, f := cl.kinds[resourceGVK]; f {
Expand Down
54 changes: 37 additions & 17 deletions pilot/pkg/config/kube/gateway/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
klabels "k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/gateway-api/apis/v1beta1"

"istio.io/istio/pilot/pkg/credentials"
"istio.io/istio/pilot/pkg/features"
Expand Down Expand Up @@ -98,11 +99,9 @@ func NewController(
) *Controller {
var ctl *status.Controller

namespaces := kclient.New[*corev1.Namespace](kc)
gatewayController := &Controller{
client: kc,
cache: c,
namespaces: namespaces,
credentialsController: credsController,
cluster: options.ClusterID,
domain: options.DomainSuffix,
Expand All @@ -112,16 +111,19 @@ func NewController(
waitForCRD: waitForCRD,
}

namespaces.AddEventHandler(controllers.EventHandler[*corev1.Namespace]{
AddFunc: func(ns *corev1.Namespace) {
gatewayController.namespaceEvent(nil, ns)
},
UpdateFunc: func(oldNs, newNs *corev1.Namespace) {
if !labels.Instance(oldNs.Labels).Equals(newNs.Labels) {
gatewayController.namespaceEvent(oldNs, newNs)
}
},
})
if !kc.IsMultiTenant() {
gatewayController.namespaces = kclient.New[*corev1.Namespace](kc)
gatewayController.namespaces.AddEventHandler(controllers.EventHandler[*corev1.Namespace]{
AddFunc: func(ns *corev1.Namespace) {
gatewayController.namespaceEvent(nil, ns)
},
UpdateFunc: func(oldNs, newNs *corev1.Namespace) {
if !labels.Instance(oldNs.Labels).Equals(newNs.Labels) {
gatewayController.namespaceEvent(oldNs, newNs)
}
},
})
}

if credsController != nil {
credsController.AddSecretHandler(gatewayController.secretEvent)
Expand Down Expand Up @@ -203,10 +205,25 @@ func (c *Controller) Reconcile(ps *model.PushContext) error {
return nil
}

nsl := c.namespaces.List("", klabels.Everything())
namespaces := make(map[string]*corev1.Namespace, len(nsl))
for _, ns := range nsl {
namespaces[ns.Name] = ns
namespaces := map[string]*corev1.Namespace{}
if c.namespaces != nil {
nsl := c.namespaces.List("", klabels.Everything())
for _, ns := range nsl {
namespaces[ns.Name] = ns
}
} else {
// we don't support namespace selectors in multi-tenant Istio right now,
// so we remove them, inducing default behavior (namespace-local)
for _, obj := range gateway {
gw := obj.Spec.(*v1beta1.GatewaySpec)
for _, listener := range gw.Listeners {
if listener.AllowedRoutes != nil &&
listener.AllowedRoutes.Namespaces != nil &&
listener.AllowedRoutes.Namespaces.Selector != nil {
listener.AllowedRoutes.Namespaces.Selector = nil
}
}
}
}
input.Namespaces = namespaces

Expand Down Expand Up @@ -281,6 +298,9 @@ func (c *Controller) RegisterEventHandler(typ config.GroupVersionKind, handler m
}

func (c *Controller) Run(stop <-chan struct{}) {
if c.namespaces == nil {
return
}
go func() {
if c.waitForCRD(gvk.GatewayClass, stop) {
gcc := NewClassController(c.client)
Expand All @@ -291,7 +311,7 @@ func (c *Controller) Run(stop <-chan struct{}) {
}

func (c *Controller) HasSynced() bool {
return c.cache.HasSynced() && c.namespaces.HasSynced()
return c.cache.HasSynced() && (c.namespaces == nil || c.namespaces.HasSynced())
}

func (c *Controller) SecretAllowed(resourceName string, namespace string) bool {
Expand Down
99 changes: 52 additions & 47 deletions pilot/pkg/config/kube/gateway/deploymentcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
klabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
gateway "sigs.k8s.io/gateway-api/apis/v1beta1"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -143,8 +144,6 @@ var knownControllers = func() sets.String {
func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *model.Environment,
webhookConfig func() inject.WebhookConfig, injectionHandler func(fn func()), tw revisions.TagWatcher, revision string,
) *DeploymentController {
gateways := kclient.New[*gateway.Gateway](client)
gatewayClasses := kclient.New[*gateway.GatewayClass](client)
dc := &DeploymentController{
client: client,
clusterID: clusterID,
Expand All @@ -159,12 +158,10 @@ func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *mode
}, subresources...)
return err
},
gateways: gateways,
gatewayClasses: gatewayClasses,
injectConfig: webhookConfig,
tagWatcher: tw,
revision: revision,
injectConfig: webhookConfig,
revision: revision,
}

dc.queue = controllers.NewQueue("gateway deployment",
controllers.WithReconciler(dc.Reconcile),
controllers.WithMaxAttempts(5))
Expand All @@ -186,23 +183,29 @@ func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *mode
dc.serviceAccounts.AddEventHandler(parentHandler)
dc.clients[gvr.ServiceAccount] = NewUntypedWrapper(dc.serviceAccounts)

dc.namespaces = kclient.New[*corev1.Namespace](client)
dc.namespaces.AddEventHandler(controllers.ObjectHandler(func(o controllers.Object) {
// TODO: make this more intelligent, checking if something we care about has changed
// requeue this namespace
for _, gw := range dc.gateways.List(o.GetName(), klabels.Everything()) {
dc.queue.AddObject(gw)
}
}))
dc.gateways = kclient.New[*gateway.Gateway](client)
dc.gateways.AddEventHandler(controllers.ObjectHandler(dc.queue.AddObject))

gateways.AddEventHandler(controllers.ObjectHandler(dc.queue.AddObject))
gatewayClasses.AddEventHandler(controllers.ObjectHandler(func(o controllers.Object) {
for _, g := range dc.gateways.List(metav1.NamespaceAll, klabels.Everything()) {
if string(g.Spec.GatewayClassName) == o.GetName() {
dc.queue.AddObject(g)
if !client.IsMultiTenant() {
dc.namespaces = kclient.New[*corev1.Namespace](client)
dc.namespaces.AddEventHandler(controllers.ObjectHandler(func(o controllers.Object) {
// TODO: make this more intelligent, checking if something we care about has changed
// requeue this namespace
for _, gw := range dc.gateways.List(o.GetName(), klabels.Everything()) {
dc.queue.AddObject(gw)
}
}
}))
}))
dc.gatewayClasses = kclient.New[*gateway.GatewayClass](client)
dc.gatewayClasses.AddEventHandler(controllers.ObjectHandler(func(o controllers.Object) {
for _, g := range dc.gateways.List(metav1.NamespaceAll, klabels.Everything()) {
if string(g.Spec.GatewayClassName) == o.GetName() {
dc.queue.AddObject(g)
}
}
}))
dc.tagWatcher = tw
dc.tagWatcher.AddHandler(dc.HandleTagChange)
}

// On injection template change, requeue all gateways
injectionHandler(func() {
Expand All @@ -211,24 +214,19 @@ func NewDeploymentController(client kube.Client, clusterID cluster.ID, env *mode
}
})

dc.tagWatcher.AddHandler(dc.HandleTagChange)

return dc
}

func (d *DeploymentController) Run(stop <-chan struct{}) {
kube.WaitForCacheSync(
stop,
d.namespaces.HasSynced,
d.deployments.HasSynced,
d.services.HasSynced,
d.serviceAccounts.HasSynced,
d.gateways.HasSynced,
d.gatewayClasses.HasSynced,
d.tagWatcher.HasSynced,
)
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)
shutdownFuncs = append(shutdownFuncs, d.namespaces, d.gatewayClasses)
}
kube.WaitForCacheSync(stop, syncFuncs...)
d.queue.Run(stop)
controllers.ShutdownAll(d.namespaces, d.deployments, d.services, d.serviceAccounts, d.gateways, d.gatewayClasses)
controllers.ShutdownAll(shutdownFuncs...)
}

// Reconcile takes in the name of a Gateway and ensures the cluster is in the desired state
Expand All @@ -243,7 +241,10 @@ func (d *DeploymentController) Reconcile(req types.NamespacedName) error {
return nil
}

gc := d.gatewayClasses.Get(string(gw.Spec.GatewayClassName), "")
var gc *gateway.GatewayClass
if d.gatewayClasses != nil {
gc = d.gatewayClasses.Get(string(gw.Spec.GatewayClassName), "")
}
if gc != nil {
// We found the gateway class, but we do not implement it. Skip
if !knownControllers.Contains(string(gc.Spec.ControllerName)) {
Expand All @@ -256,18 +257,22 @@ func (d *DeploymentController) Reconcile(req types.NamespacedName) error {
}
}

// 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 {
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 {
return nil
}
selectedTag = ns.Labels[label.IoIstioRev.Name]
}
if d.tagWatcher != nil {
myTags := d.tagWatcher.GetMyTags()
if !myTags.Contains(selectedTag) && !(selectedTag == "" && myTags.Contains("default")) {
return nil
}
}
selectedTag = ns.Labels[label.IoIstioRev.Name]
}
myTags := d.tagWatcher.GetMyTags()
if !myTags.Contains(selectedTag) && !(selectedTag == "" && myTags.Contains("default")) {
return nil
}
// TODO: Here we could check if the tag is set and matches no known tags, and handle that if we are default.

Expand Down
10 changes: 0 additions & 10 deletions pilot/pkg/config/kube/gateway/testdata/deployment/cluster-ip.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,6 @@ spec:
periodSeconds: 15
successThreshold: 1
timeoutSeconds: 1
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsGroup: 1337
runAsNonRoot: true
runAsUser: 1337
startupProbe:
failureThreshold: 30
httpGet:
Expand Down
10 changes: 0 additions & 10 deletions pilot/pkg/config/kube/gateway/testdata/deployment/manual-ip.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,6 @@ spec:
periodSeconds: 15
successThreshold: 1
timeoutSeconds: 1
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsGroup: 1337
runAsNonRoot: true
runAsUser: 1337
startupProbe:
failureThreshold: 30
httpGet:
Expand Down
10 changes: 0 additions & 10 deletions pilot/pkg/config/kube/gateway/testdata/deployment/manual-sa.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,6 @@ spec:
periodSeconds: 15
successThreshold: 1
timeoutSeconds: 1
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsGroup: 1337
runAsNonRoot: true
runAsUser: 1337
startupProbe:
failureThreshold: 30
httpGet:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,6 @@ spec:
periodSeconds: 15
successThreshold: 1
timeoutSeconds: 1
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsGroup: 1337
runAsNonRoot: true
runAsUser: 1337
startupProbe:
failureThreshold: 30
httpGet:
Expand Down
Loading

0 comments on commit e466ba0

Please sign in to comment.