Skip to content

Commit

Permalink
chore(meshgateway): forbid possibility of setting kuma.io/service for…
Browse files Browse the repository at this point in the history
… MGI (#10968)

Signed-off-by: Lukasz Dziedziak <[email protected]>
  • Loading branch information
lukidzi authored Jul 26, 2024
1 parent 52c4076 commit 84a6377
Show file tree
Hide file tree
Showing 24 changed files with 125 additions and 126 deletions.
28 changes: 28 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,34 @@ If the `--kuma-dp-user` flag is not provided, the system will attempt to use the

Please update your scripts and configurations accordingly to accommodate this change.

### Setting `kuma.io/service` in tags of `MeshGatewayInstance` had been forbidden

To increase security, in version 2.7.x, setting a `kuma.io/service` tag for the `MeshGatewayInstance` was deprecated and since 2.9.x is not supported. We generate the `kuma.io/service` tag based on the `MeshGatewayInstance` resource. The service name is constructed as `{MeshGatewayInstance name}_{MeshGatewayInstance namespace}_svc`.

E.g.:

```yaml
apiVersion: kuma.io/v1alpha1
kind: MeshGatewayInstance
metadata:
name: demo-app
namespace: kuma-demo
labels:
kuma.io/mesh: default
```

The generated `kuma.io/service` value is `demo-app_kuma-demo_svc`.

#### Migration

The migration process requires updating all policies and `MeshGateway` resources using the old `kuma.io/service` value to adopt the new one.

Migration step:
1. Create a copy of policies using the new `kuma.io/service` and the new resource name to avoid overwriting previous policies.
2. Duplicate the `MeshGateway` resource with a selector using the new `kuma.io/service` value.
3. Deploy the gateway and verify if traffic works correctly.
4. Remove the old resources.

### kumactl

#### Default prometheus scrape config removes `service`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1423,8 +1423,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1423,8 +1423,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1443,8 +1443,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
4 changes: 2 additions & 2 deletions app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3027,8 +3027,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
4 changes: 2 additions & 2 deletions docs/generated/raw/crds/kuma.io_meshgatewayinstances.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ spec:
type: string
description: |-
Tags specifies the Kuma tags that are propagated to the managed
dataplane proxies. These tags should include exactly one
`kuma.io/service` tag, and should match exactly one Gateway
dataplane proxies. These tags should not include `kuma.io/service` tag
since is auto-generated, and should match exactly one Gateway
resource.
type: object
type: object
Expand Down
4 changes: 4 additions & 0 deletions pkg/core/resources/apis/mesh/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type (
type ValidateTagsOpts struct {
RequireAtLeastOneTag bool
RequireService bool
ForbidService bool
ExtraTagsValidators []TagsValidatorFunc
ExtraTagKeyValidators []TagKeyValidatorFunc
ExtraTagValueValidators []TagValueValidatorFunc
Expand Down Expand Up @@ -135,6 +136,9 @@ func validateTagKeyValues(path validators.PathBuilder, keyValues map[string]stri
}
}
_, defined := keyValues[mesh_proto.ServiceTag]
if opts.ForbidService && defined {
err.AddViolationAt(path, fmt.Sprintf("%q must not be defined", mesh_proto.ServiceTag))
}
if opts.RequireService && !defined {
err.AddViolationAt(path, fmt.Sprintf("mandatory tag %q is missing", mesh_proto.ServiceTag))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ type MeshGatewayInstanceSpec struct {
MeshGatewayCommonConfig `json:",inline"`

// Tags specifies the Kuma tags that are propagated to the managed
// dataplane proxies. These tags should include exactly one
// `kuma.io/service` tag, and should match exactly one Gateway
// dataplane proxies. These tags should not include `kuma.io/service` tag
// since is auto-generated, and should match exactly one Gateway
// resource.
//
// +required
Expand Down
38 changes: 2 additions & 36 deletions pkg/plugins/runtime/k8s/controllers/gateway_instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ func (r *GatewayInstanceReconciler) Reconcile(ctx context.Context, req kube_ctrl
return kube_ctrl.Result{}, errors.Wrap(err, "unable to get Namespace of MeshGatewayInstance")
}

if len(gatewayInstance.Spec.Tags) > 0 && gatewayInstance.Spec.Tags[mesh_proto.ServiceTag] != "" {
r.Log.Info("WARNING: Setting kuma.io/service tag is deprecated for this object kind, control-plane creates a name based on the resource name and namespace",
"name", gatewayInstance.GetName(), "namespace", gatewayInstance.GetNamespace(), "kind", gatewayInstance.GetObjectKind().GroupVersionKind().Kind)
}

mesh := k8s_util.MeshOfByLabelOrAnnotation(r.Log, gatewayInstance, &ns)

orig := gatewayInstance.DeepCopy()
Expand Down Expand Up @@ -106,7 +101,7 @@ func k8sSelector(name string) map[string]string {
return map[string]string{"app": name}
}

func (_ *GatewayInstanceReconciler) gatewayInstanceTags(gatewayInstance *mesh_k8s.MeshGatewayInstance) map[string]string {
func (*GatewayInstanceReconciler) gatewayInstanceTags(gatewayInstance *mesh_k8s.MeshGatewayInstance) map[string]string {
tags := maps.Clone(gatewayInstance.Spec.Tags)
if tags == nil {
tags = map[string]string{}
Expand Down Expand Up @@ -420,9 +415,7 @@ func updateStatus(
)
}

const serviceKey string = ".metadata.service"

// GatewayToInstanceMapper maps a Gateway object to MeshGatewayInstance objects by
// GatewayToInstanceMapper maps a MeshGateway object to MeshGatewayInstance objects by
// using the service tag to list GatewayInstances with a matching index.
// The index is set up on MeshGatewayInstance in SetupWithManager and holds the service
// tag from the MeshGatewayInstance tags.
Expand All @@ -447,22 +440,6 @@ func GatewayToInstanceMapper(l logr.Logger, client kube_client.Client) kube_hand

var req []kube_reconcile.Request
for _, serviceName := range serviceNames {
// TODO: remove in version 2.9.x
// get MeshGatewayInstance by kuma.io/service name and map to requests
instances := &mesh_k8s.MeshGatewayInstanceList{}
err := client.List(ctx, instances, kube_client.MatchingFields{serviceKey: serviceName})
if err != nil && !kube_apierrs.IsNotFound(err) {
l.Error(err, "failed to fetch GatewayInstances")
}
if len(instances.Items) > 0 {
for _, instance := range instances.Items {
req = append(req, kube_reconcile.Request{
NamespacedName: kube_types.NamespacedName{Namespace: instance.Namespace, Name: instance.Name},
})
}
continue
}

// map generated kuma.io/service to namespaced name
name, err := k8s_util.NamespacesNameFromServiceTag(serviceName)
if err != nil {
Expand Down Expand Up @@ -492,17 +469,6 @@ func (r *GatewayInstanceReconciler) SetupWithManager(mgr kube_ctrl.Manager) erro
return err
}

// TODO: remove in version 2.9.x
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &mesh_k8s.MeshGatewayInstance{}, serviceKey, func(obj kube_client.Object) []string {
instance := obj.(*mesh_k8s.MeshGatewayInstance)

serviceName := instance.Spec.Tags[mesh_proto.ServiceTag]

return []string{serviceName}
}); err != nil {
return err
}

return kube_ctrl.NewControllerManagedBy(mgr).
For(&mesh_k8s.MeshGatewayInstance{}).
Owns(&kube_core.Service{}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ func (h *GatewayInstanceValidator) ValidateUpdate(ctx context.Context, req admis
func (h *GatewayInstanceValidator) validateTags(gatewayInstance *mesh_k8s.MeshGatewayInstance) admission.Response {
tags := gatewayInstance.Spec.Tags

err := core_mesh.ValidateTags(validators.RootedAt("tags"), tags, core_mesh.ValidateTagsOpts{})

err := core_mesh.ValidateTags(validators.RootedAt("tags"), tags, core_mesh.ValidateTagsOpts{ForbidService: true})
if err.HasViolations() {
return convertValidationErrorOf(err, gatewayInstance, gatewayInstance.GetObjectMeta())
}
Expand Down
55 changes: 37 additions & 18 deletions pkg/plugins/runtime/k8s/webhooks/gateway_instance_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,10 @@ var _ = Describe("MeshGatewayInstanceValidator", func() {
labels:
kuma.io/mesh: default
spec:
selectors:
- match:
kuma.io/service: edge-gateway_kuma-demo_svc
conf:
listeners:
- port: 8080
protocol: HTTP
tags:
port: http/8080
tags:
custom.io/gateway: my-gateway
replicas: 1
serviceType: LoadBalancer
operation: CREATE
`,
expectedMessage: "",
Expand All @@ -100,19 +95,43 @@ var _ = Describe("MeshGatewayInstanceValidator", func() {
labels:
kuma.io/mesh: default
spec:
selectors:
- match:
kuma.io/service: edge-gateway_kuma-demo_svc
conf:
listeners:
- port: 8080
protocol: HTTP
tags:
port: http/8080
tags:
custom.io/gateway: my-gateway
replicas: 1
serviceType: LoadBalancer
operation: CREATE
`,
expectedMessage: "Operation not allowed. Kuma resources like MeshGatewayInstance can be created only from the 'zone' control plane and not from a 'global' control plane.",
}),

Entry("should not allow create MeshGatewayInstance with kuma.io/service tag", testCase{
cpMode: core.Zone,
request: `
apiVersion: admission.k8s.io/v1
kind: AdmissionReview
request:
uid: 12345
kind:
group: "kuma.io"
kind: "MeshGatewayInstance"
version: v1alpha1
name: my-gateway
object:
apiVersion: kuma.io/v1alpha1
kind: MeshGatewayInstance
metadata:
name: my-gateway
labels:
kuma.io/mesh: default
spec:
tags:
kuma.io/service: edge-gateway_kuma-demo_svc
replicas: 1
serviceType: LoadBalancer
operation: CREATE
`,
expectedMessage: "tags: \"kuma.io/service\" must not be defined",
}),
)
})

Expand Down
4 changes: 2 additions & 2 deletions test/e2e_env/kubernetes/gateway/cross-mesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ func CrossMeshGatewayOnKubernetes() {

Context("when mTLS is enabled", func() {
crossMeshGatewayYaml := mkGateway(
crossMeshGatewayName, crossMeshGatewayName, gatewayMesh, true, crossMeshHostname, echoServerService(gatewayMesh, gatewayTestNamespace), crossMeshGatewayPort,
crossMeshGatewayName, fmt.Sprintf("%s_%s_svc", crossMeshGatewayName, gatewayTestNamespace), gatewayMesh, true, crossMeshHostname, echoServerService(gatewayMesh, gatewayTestNamespace), crossMeshGatewayPort,
)
crossMeshGatewayInstanceYaml := MkGatewayInstance(crossMeshGatewayName, gatewayTestNamespace, gatewayMesh)
edgeGatewayYaml := mkGateway(
edgeGatewayName, edgeGatewayName, gatewayOtherMesh, false, "", echoServerService(gatewayOtherMesh, gatewayTestNamespace), edgeGatewayPort,
edgeGatewayName, fmt.Sprintf("%s_%s_svc", edgeGatewayName, gatewayTestNamespace), gatewayOtherMesh, false, "", echoServerService(gatewayOtherMesh, gatewayTestNamespace), edgeGatewayPort,
)
edgeGatewayInstanceYaml := MkGatewayInstance(
edgeGatewayName, gatewayTestNamespace, gatewayOtherMesh,
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_env/kubernetes/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ type: system.kuma.io/secret
)).
Install(YamlK8s(httpsSecret())).
Install(YamlK8s(meshGateway)).
Install(YamlK8s(MkGatewayInstanceNoServiceTag("simple-gateway", namespace, meshName))).
Install(YamlK8s(MkGatewayInstance("simple-gateway", namespace, meshName))).
Install(MeshTrafficPermissionAllowAllKubernetes(meshName)).
Setup(kubernetes.Cluster)
Expect(err).ToNot(HaveOccurred())
Expand Down
15 changes: 8 additions & 7 deletions test/e2e_env/kubernetes/gateway/mtls.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mesh: gateway-mtls
spec:
selectors:
- match:
kuma.io/service: mtls-edge-gateway
kuma.io/service: mtls-edge-gateway_gateway-mtls_svc
conf:
listeners:
- port: 8080
Expand Down Expand Up @@ -113,7 +113,7 @@ mesh: gateway-mtls
spec:
selectors:
- match:
kuma.io/service: mtls-edge-gateway
kuma.io/service: mtls-edge-gateway_gateway-mtls_svc
hostname: example.kuma.io
conf:
http:
Expand Down Expand Up @@ -318,8 +318,9 @@ spec:
name: non-accessible-echo-server_gateway-mtls_svc_80
from:
- targetRef:
kind: MeshService
name: not-mtls-edge-gateway
kind: MeshSubset
tags:
kuma.io/service: not-mtls-edge-gateway_gateway-mtls_svc
default:
action: Allow`
Expect(kubernetes.Cluster.Install(YamlK8s(tp))).To(Succeed())
Expand Down Expand Up @@ -347,7 +348,7 @@ mesh: gateway-mtls
spec:
selectors:
- match:
kuma.io/service: mtls-edge-gateway
kuma.io/service: mtls-edge-gateway_gateway-mtls_svc
protocol: tcp
conf:
tcp:
Expand Down Expand Up @@ -393,7 +394,7 @@ mesh: gateway-mtls
spec:
selectors:
- match:
kuma.io/service: mtls-edge-gateway
kuma.io/service: mtls-edge-gateway_gateway-mtls_svc
name: tls-passthrough
conf:
tcp:
Expand All @@ -411,7 +412,7 @@ mesh: gateway-mtls
spec:
selectors:
- match:
kuma.io/service: mtls-edge-gateway
kuma.io/service: mtls-edge-gateway_gateway-mtls_svc
name: tls-terminate
conf:
tcp:
Expand Down
Loading

0 comments on commit 84a6377

Please sign in to comment.