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.

Sign up for GitHub

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

only add istio automtls when label has value #10574

Merged
merged 5 commits into from
Jan 17, 2025
Merged

only add istio automtls when label has value #10574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2854371
only add istio automtls when label has value
stevenctl Jan 13, 2025
025603e
Merge branch 'main' into stevenctl/fix-automtls-detection
stevenctl Jan 15, 2025
30dbdb5
changelog
stevenctl Jan 15, 2025
712487c
link to Istio API def
stevenctl Jan 16, 2025
3958c1f
move changelog
stevenctl Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/v1.19.0-beta4/istio-automtls-disabled.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/10575
resolvesIssue: true
description: |
When a workload has the label `security.istio.io/tlsMode: disabled`
we will no longer attempt to send mTLS to that workload.
22 changes: 2 additions & 20 deletions projects/gateway2/krtcollections/endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

ggv2utils "github.com/solo-io/gloo/projects/gateway2/utils"
"github.com/solo-io/gloo/projects/gloo/constants"
v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
glookubev1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/kube/apis/gloo.solo.io/v1"
kubeplugin "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/kubernetes"
"github.com/solo-io/gloo/projects/gloo/pkg/plugins/istio_automtls"
"github.com/solo-io/gloo/projects/gloo/pkg/translator"
"github.com/solo-io/go-utils/contextutils"
"istio.io/istio/pkg/kube"
Expand Down Expand Up @@ -336,7 +336,7 @@ func CreateLBEndpoint(address string, port uint32, podLabels map[string]string,
metadata := &envoy_config_core_v3.Metadata{
FilterMetadata: map[string]*structpb.Struct{},
}
metadata = addIstioAutomtlsMetadata(metadata, podLabels, enableAutoMtls)
metadata = istio_automtls.AddIstioAutomtlsMetadata(metadata, podLabels, enableAutoMtls)
// Don't add the annotations to the metadata - it's not documented so it's not coming
// metadata = addAnnotations(metadata, addr.GetMetadata().GetAnnotations())

Expand Down Expand Up @@ -365,24 +365,6 @@ func CreateLBEndpoint(address string, port uint32, podLabels map[string]string,
}
}

func addIstioAutomtlsMetadata(metadata *envoy_config_core_v3.Metadata, labels map[string]string, enableAutoMtls bool) *envoy_config_core_v3.Metadata {
const EnvoyTransportSocketMatch = "envoy.transport_socket_match"
if enableAutoMtls {
if _, ok := labels[constants.IstioTlsModeLabel]; ok {
metadata.GetFilterMetadata()[EnvoyTransportSocketMatch] = &structpb.Struct{
Fields: map[string]*structpb.Value{
constants.TLSModeLabelShortname: {
Kind: &structpb.Value_StringValue{
StringValue: constants.IstioMutualTLSModeLabel,
},
},
},
}
}
}
return metadata
}

func findPortForService(kctx krt.HandlerContext, services krt.Collection[*corev1.Service], spec *kubeplugin.UpstreamSpec) (*corev1.ServicePort, bool) {
maybeSvc := krt.FetchOne(kctx, services, krt.FilterObjectName(types.NamespacedName{
Namespace: spec.GetServiceNamespace(),
Expand Down
36 changes: 36 additions & 0 deletions projects/gloo/pkg/plugins/istio_automtls/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package istio_automtls

import (
"github.com/solo-io/gloo/projects/gloo/constants"
"google.golang.org/protobuf/types/known/structpb"

envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)

const EnvoyTransportSocketMatch = "envoy.transport_socket_match"

// AddIstioAutomtlsMetadata adds metadata used by the transport_socket_match
// to select the mTLS transport socket. The Envoy metadata label is added
// based on the presence of the Istio workload label "security.istio.io/tlsMode=istio".
func AddIstioAutomtlsMetadata(
metadata *envoy_config_core_v3.Metadata,
workloadLabels map[string]string,
enableAutoMtls bool,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I know this is a copy/paste of an existing function, so I'm happy to leave it as is. It feels strange to me that we pass a boolean and only perform an action if that's true. It feels like we could just have the function only be called if the enableAutoMtls value is true

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, I'd rather have "pass the global setting in, let some shared code interpret it" than have that conditional in multiple places, as easy as it may be.

) *envoy_config_core_v3.Metadata {
if enableAutoMtls {
// Valid label values are 'istio', 'disabled'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add a link to the istio ref for these values? I already have forgotten the istio semantics regarding these

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also just for for clarity i was thinking adding this link to the comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linked to the Istio API def that outlines it.

// https://github.com/istio/api/blob/5b3f065ee1c2802fb4bc6010ac847c181caa6cc3/label/labels.gen.go#L285
if value, ok := workloadLabels[constants.IstioTlsModeLabel]; ok && value == constants.IstioMutualTLSModeLabel {
metadata.GetFilterMetadata()[EnvoyTransportSocketMatch] = &structpb.Struct{
Fields: map[string]*structpb.Value{
constants.TLSModeLabelShortname: {
Kind: &structpb.Value_StringValue{
StringValue: constants.IstioMutualTLSModeLabel,
},
},
},
}
}
}
return metadata
}