diff --git a/control-plane/api-gateway/gatekeeper/init.go b/control-plane/api-gateway/gatekeeper/init.go index f861e6ddfa..76047beaaa 100644 --- a/control-plane/api-gateway/gatekeeper/init.go +++ b/control-plane/api-gateway/gatekeeper/init.go @@ -191,12 +191,12 @@ func (g Gatekeeper) initContainer(config common.HelmConfig, name, namespace stri return corev1.Container{}, fmt.Errorf("error getting namespace metadata for deployment: %s", err) } - uid, err = ctrlCommon.GetOpenShiftUID(ns) + uid, err = ctrlCommon.GetOpenShiftUID(ns, ctrlCommon.SelectFirstInRange) if err != nil { return corev1.Container{}, err } - group, err = ctrlCommon.GetOpenShiftGroup(ns) + group, err = ctrlCommon.GetOpenShiftGroup(ns, ctrlCommon.SelectFirstInRange) if err != nil { return corev1.Container{}, err } diff --git a/control-plane/connect-inject/common/openshift.go b/control-plane/connect-inject/common/openshift.go index e8e2f555e8..c978bdde2b 100644 --- a/control-plane/connect-inject/common/openshift.go +++ b/control-plane/connect-inject/common/openshift.go @@ -20,12 +20,14 @@ import ( "strconv" "strings" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" corev1 "k8s.io/api/core/v1" + + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" ) // GetOpenShiftUID gets the user id from the OpenShift annotation 'openshift.io/sa.scc.uid-range'. -func GetOpenShiftUID(ns *corev1.Namespace) (int64, error) { +// Select the last in the range so we don't conflict with any ID assigned to application containers. +func GetOpenShiftUID(ns *corev1.Namespace, selector idSelector) (int64, error) { annotation, ok := ns.Annotations[constants.AnnotationOpenShiftUIDRange] if !ok { return 0, fmt.Errorf("unable to find annotation %s", constants.AnnotationOpenShiftUIDRange) @@ -34,7 +36,7 @@ func GetOpenShiftUID(ns *corev1.Namespace) (int64, error) { return 0, fmt.Errorf("found annotation %s but it was empty", constants.AnnotationOpenShiftUIDRange) } - uid, err := parseOpenShiftUID(annotation) + uid, err := parseOpenShiftUID(annotation, selector) if err != nil { return 0, err } @@ -45,15 +47,11 @@ func GetOpenShiftUID(ns *corev1.Namespace) (int64, error) { // parseOpenShiftUID parses the UID "range" from the annotation string. The annotation can either have a '/' or '-' // as a separator. '-' is the old style of UID from when it used to be an actual range. // Example annotation value: "1000700000/100000". -func parseOpenShiftUID(val string) (int64, error) { +func parseOpenShiftUID(val string, selector idSelector) (int64, error) { var uid int64 var err error if strings.Contains(val, "/") { - str := strings.Split(val, "/") - uid, err = strconv.ParseInt(str[0], 10, 64) - if err != nil { - return 0, err - } + return selectIDInRange(val, selector) } if strings.Contains(val, "-") { str := strings.Split(val, "-") @@ -77,7 +75,8 @@ func parseOpenShiftUID(val string) (int64, error) { // GetOpenShiftGroup gets the group from OpenShift annotation 'openshift.io/sa.scc.supplemental-groups' // Fall back to the UID annotation if the group annotation does not exist. The values should // be the same. -func GetOpenShiftGroup(ns *corev1.Namespace) (int64, error) { +// Select the last in the range so we don't conflict with any ID assigned randomly to application containers. +func GetOpenShiftGroup(ns *corev1.Namespace, selector idSelector) (int64, error) { annotation, ok := ns.Annotations[constants.AnnotationOpenShiftGroups] if !ok { // fall back to UID annotation @@ -94,25 +93,21 @@ func GetOpenShiftGroup(ns *corev1.Namespace) (int64, error) { return 0, fmt.Errorf("found annotation %s but it was empty", constants.AnnotationOpenShiftGroups) } - uid, err := parseOpenShiftGroup(annotation) + gid, err := parseOpenShiftGroup(annotation, selector) if err != nil { return 0, err } - return uid, nil + return gid, nil } // parseOpenShiftGroup parses the group from the annotation string. The annotation can either have a '/' or ',' // as a separator. ',' is the old style of UID from when it used to be an actual range. -func parseOpenShiftGroup(val string) (int64, error) { +func parseOpenShiftGroup(val string, selector idSelector) (int64, error) { var group int64 var err error if strings.Contains(val, "/") { - str := strings.Split(val, "/") - group, err = strconv.ParseInt(str[0], 10, 64) - if err != nil { - return 0, err - } + return selectIDInRange(val, selector) } if strings.Contains(val, ",") { str := strings.Split(val, ",") @@ -128,3 +123,50 @@ func parseOpenShiftGroup(val string) (int64, error) { return group, nil } + +type idSelector func(values []int64) (int64, error) + +var SelectFirstInRange idSelector = func(values []int64) (int64, error) { + if len(values) < 1 { + return 0, fmt.Errorf("range must have at least 1 value") + } + return values[0], nil +} + +var SelectSidecarID idSelector = func(values []int64) (int64, error) { + if len(values) < 2 { + return 0, fmt.Errorf("range must have at least 2 values") + } + return values[len(values)-2], nil +} + +var SelectInitContainerID idSelector = func(values []int64) (int64, error) { + if len(values) < 1 { + return 0, fmt.Errorf("range must have at least 1 value") + } + return values[len(values)-1], nil +} + +func selectIDInRange(value string, selector idSelector) (int64, error) { + parts := strings.Split(value, "/") + if len(parts) != 2 { + return 0, fmt.Errorf("invalid range format: %s", value) + } + + start, err := strconv.Atoi(parts[0]) + if err != nil { + return 0, fmt.Errorf("invalid range format: %s", parts[0]) + } + + length, err := strconv.Atoi(parts[1]) + if err != nil { + return 0, fmt.Errorf("invalid range format: %s", parts[1]) + } + + values := make([]int64, length) + for i := 0; i < length; i++ { + values[i] = int64(start + i) + } + + return selector(values) +} diff --git a/control-plane/connect-inject/common/openshift_test.go b/control-plane/connect-inject/common/openshift_test.go index e4a5178c7a..2f25e3a6e8 100644 --- a/control-plane/connect-inject/common/openshift_test.go +++ b/control-plane/connect-inject/common/openshift_test.go @@ -6,10 +6,11 @@ import ( "fmt" "testing" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" ) func TestOpenShiftUID(t *testing.T) { @@ -110,7 +111,7 @@ func TestOpenShiftUID(t *testing.T) { for _, tt := range cases { t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - actual, err := GetOpenShiftUID(tt.Namespace()) + actual, err := GetOpenShiftUID(tt.Namespace(), SelectFirstInRange) if tt.Err == "" { require.NoError(err) require.Equal(tt.Expected, actual) @@ -224,7 +225,7 @@ func TestOpenShiftGroup(t *testing.T) { for _, tt := range cases { t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - actual, err := GetOpenShiftGroup(tt.Namespace()) + actual, err := GetOpenShiftGroup(tt.Namespace(), SelectFirstInRange) if tt.Err == "" { require.NoError(err) require.Equal(tt.Expected, actual) diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go index e13e287ba3..aba6d5ded9 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go @@ -243,11 +243,11 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor // Transparent proxy is set in OpenShift. There is an annotation on the namespace that tells us what // the user and group ids should be for the sidecar. var err error - uid, err = common.GetOpenShiftUID(&namespace) + uid, err = common.GetOpenShiftUID(&namespace, common.SelectSidecarID) if err != nil { return corev1.Container{}, err } - group, err = common.GetOpenShiftGroup(&namespace) + group, err = common.GetOpenShiftGroup(&namespace, common.SelectSidecarID) if err != nil { return corev1.Container{}, err } diff --git a/control-plane/connect-inject/webhook/container_init.go b/control-plane/connect-inject/webhook/container_init.go index 4138441505..b634aff48b 100644 --- a/control-plane/connect-inject/webhook/container_init.go +++ b/control-plane/connect-inject/webhook/container_init.go @@ -10,10 +10,11 @@ import ( "strings" "text/template" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common" - "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" corev1 "k8s.io/api/core/v1" "k8s.io/utils/pointer" + + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common" + "github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants" ) const ( @@ -240,12 +241,12 @@ func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod, if w.EnableOpenShift { var err error - uid, err = common.GetOpenShiftUID(&namespace) - + uid, err = common.GetOpenShiftUID(&namespace, common.SelectInitContainerID) if err != nil { return corev1.Container{}, err } - group, err = common.GetOpenShiftGroup(&namespace) + + group, err = common.GetOpenShiftGroup(&namespace, common.SelectInitContainerID) if err != nil { return corev1.Container{}, err } diff --git a/control-plane/connect-inject/webhook/redirect_traffic.go b/control-plane/connect-inject/webhook/redirect_traffic.go index e6de09f448..25e089514b 100644 --- a/control-plane/connect-inject/webhook/redirect_traffic.go +++ b/control-plane/connect-inject/webhook/redirect_traffic.go @@ -31,14 +31,23 @@ func (w *MeshWebhook) iptablesConfigJSON(pod corev1.Pod, ns corev1.Namespace) (s if !w.EnableOpenShift { cfg.ProxyUserID = strconv.Itoa(sidecarUserAndGroupID) + + // Add init container user ID to exclude from traffic redirection. + cfg.ExcludeUIDs = append(cfg.ExcludeUIDs, strconv.Itoa(initContainersUserAndGroupID)) } else { // When using OpenShift, the uid and group are saved as an annotation on the namespace - uid, err := common.GetOpenShiftUID(&ns) + uid, err := common.GetOpenShiftUID(&ns, common.SelectSidecarID) if err != nil { return "", err } cfg.ProxyUserID = strconv.FormatInt(uid, 10) + // Exclude the user ID for the init container from traffic redirection. + uid, err = common.GetOpenShiftGroup(&ns, common.SelectInitContainerID) + if err != nil { + return "", err + } + cfg.ExcludeUIDs = append(cfg.ExcludeUIDs, strconv.FormatInt(uid, 10)) } // Set the proxy's inbound port. @@ -110,9 +119,6 @@ func (w *MeshWebhook) iptablesConfigJSON(pod corev1.Pod, ns corev1.Namespace) (s excludeUIDs := splitCommaSeparatedItemsFromAnnotation(constants.AnnotationTProxyExcludeUIDs, pod) cfg.ExcludeUIDs = append(cfg.ExcludeUIDs, excludeUIDs...) - // Add init container user ID to exclude from traffic redirection. - cfg.ExcludeUIDs = append(cfg.ExcludeUIDs, strconv.Itoa(initContainersUserAndGroupID)) - dnsEnabled, err := consulDNSEnabled(ns, pod, w.EnableConsulDNS, w.EnableTransparentProxy) if err != nil { return "", err