Add DisableHostportMapping option to configuration,

allowing users to disable hostport mapping for pods,
which can be useful when other services provide it other than kube-proxy (like Cillium)

Closes: cri-o#3914

Signed-off-by: T K Chandra Hasan <t.k.chandra.hasan@ibm.com>

completions/bash/crio

@@ -44,6 +44,7 @@ h
 --default-transport
 --default-ulimits
 --device-ownership-from-security-context
+--disable-hostport-mapping
 --drop-infra-ctr
 --enable-criu-support
 --enable-metrics

completions/fish/crio.fish

@@ -49,6 +49,7 @@ complete -c crio -n '__fish_crio_no_subcommand' -f -l default-sysctls -r -d 'Sys
 complete -c crio -n '__fish_crio_no_subcommand' -f -l default-transport -r -d 'A prefix to prepend to image names that cannot be pulled as-is.'
 complete -c crio -n '__fish_crio_no_subcommand' -f -l default-ulimits -r -d 'Ulimits to apply to containers by default (name=soft:hard).'
 complete -c crio -n '__fish_crio_no_subcommand' -f -l device-ownership-from-security-context -d 'Set devices\' uid/gid ownership from runAsUser/runAsGroup.'
+complete -c crio -n '__fish_crio_no_subcommand' -f -l disable-hostport-mapping -d 'If true, CRI-O would disable the hostport mapping.'
 complete -c crio -n '__fish_crio_no_subcommand' -f -l drop-infra-ctr -d 'Determines whether pods are created without an infra container, when the pod is not using a pod level PID namespace.'
 complete -c crio -n '__fish_crio_no_subcommand' -f -l enable-criu-support -d 'Enable CRIU integration, requires that the criu binary is available in $PATH.'
 complete -c crio -n '__fish_crio_no_subcommand' -f -l enable-metrics -d 'Enable metrics endpoint for the server on localhost:9090.'

completions/zsh/_crio

@@ -51,6 +51,7 @@ it later with **--config**. Global options will modify the output.'
         '--default-transport'
         '--default-ulimits'
         '--device-ownership-from-security-context'
+        '--disable-hostport-mapping'
         '--drop-infra-ctr'
         '--enable-criu-support'
         '--enable-metrics'

docs/crio.8.md

@@ -42,6 +42,7 @@ crio
 [--default-transport]=[value]
 [--default-ulimits]=[value]
 [--device-ownership-from-security-context]
+[--disable-hostport-mapping]
 [--drop-infra-ctr]
 [--enable-criu-support]
 [--enable-metrics]
@@ -218,6 +219,8 @@ crio [GLOBAL OPTIONS] command [COMMAND OPTIONS] [ARGUMENTS...]
 
 **--device-ownership-from-security-context**: Set devices' uid/gid ownership from runAsUser/runAsGroup.
 
+**--disable-hostport-mapping**: If true, CRI-O would disable the hostport mapping.
+
 **--drop-infra-ctr**: Determines whether pods are created without an infra container, when the pod is not using a pod level PID namespace.
 
 **--enable-criu-support**: Enable CRIU integration, requires that the criu binary is available in $PATH.

docs/crio.conf.5.md

@@ -300,6 +300,9 @@ Enable CRI-O to generate the container pod-level events in order to optimize the
 **hostnetwork_disable_selinux**=true
  Determines whether SELinux should be disabled within a pod when it is running in the host network namespace.
 
+**disable_hostport_mapping**=false
+ Enable/Disable the container hostport mapping in CRI-O. Default value is set to 'false'.
+
 ### CRIO.RUNTIME.RUNTIMES TABLE
 The "crio.runtime.runtimes" table defines a list of OCI compatible runtimes.  The runtime to use is picked based on the runtime handler provided by the CRI.  If no runtime handler is provided, the runtime will be picked based on the level of trust of the workload. This option supports live configuration reload. This option supports live configuration reload.
 

internal/criocli/criocli.go

@@ -403,6 +403,9 @@ func mergeConfig(config *libconfig.Config, ctx *cli.Context) error {
 	}
 	if ctx.IsSet("hostnetwork-disable-selinux") {
 		config.HostNetworkDisableSELinux = ctx.Bool("hostnetwork-disable-selinux")
+    }
+	if ctx.IsSet("disable-hostport-mapping") {
+		config.DisableHostPortMapping = ctx.Bool("disable-hostport-mapping")
 	}
 	return nil
 }
@@ -1122,6 +1125,12 @@ func getCrioFlags(defConf *libconfig.Config) []cli.Flag {
 			Usage:   "Determines whether SELinux should be disabled within a pod when it is running in the host network namespace.",
 			EnvVars: []string{"CONTAINER_HOSTNETWORK_DISABLE_SELINUX"},
 			Value:   defConf.HostNetworkDisableSELinux,
+        },
+		&cli.BoolFlag{
+			Name:    "disable-hostport-mapping",
+			Usage:   "If true, CRI-O would disable the hostport mapping.",
+			EnvVars: []string{"DISABLE_HOSTPORT_MAPPING"},
+			Value:   defConf.DisableHostPortMapping,
 		},
 	}
 }

internal/criocli/criocli_test.go

@@ -133,4 +133,30 @@ var _ = t.Describe("CLI Flags", func() {
 		// Then
 		Expect(config.RuntimeConfig.HostNetworkDisableSELinux).To(Equal(setFlag.Value))
 	})
+
+	It("Flag test disable-hostport-mapping", func() {
+		// Default Config
+		app.Flags, app.Metadata, err = criocli.GetFlagsAndMetadata()
+		Expect(err).To(BeNil())
+		config, err := criocli.GetConfigFromContext(ctx)
+		Expect(err).To(BeNil())
+
+		// Then
+		Expect(config.RuntimeConfig.DisableHostPortMapping).To(Equal(false))
+
+		// Set Config & Merge
+		setFlag := &cli.BoolFlag{
+			Name:       "disable-hostport-mapping",
+			Value:      true,
+			HasBeenSet: true,
+		}
+		err = setFlag.Apply(flagSet)
+		Expect(err).To(BeNil())
+		ctx.Command.Flags = append(commandFlags, setFlag)
+		config, err = criocli.GetAndMergeConfigFromContext(ctx)
+		Expect(err).To(BeNil())
+
+		// Then
+		Expect(config.RuntimeConfig.DisableHostPortMapping).To(Equal(true))
+	})
 })

internal/hostport/noop_hostport_manager.go

@@ -0,0 +1,22 @@
+package hostport
+
+import "github.com/sirupsen/logrus"
+
+// This interface implements, when hostport mapping is disabled in CRI-O
+type noopHostportManager struct{}
+
+// NewNoopHostportManager creates a new HostPortManager
+func NewNoopHostportManager() HostPortManager {
+	logrus.Info("HostPort Mapping is Disabled in CRI-O")
+	return &noopHostportManager{}
+}
+
+func (mh *noopHostportManager) Add(id string, podPortMapping *PodPortMapping, natInterfaceName string) error {
+	logrus.Debug("HostPort Mapping is Disabled in CRI-O")
+	return nil
+}
+
+func (mh *noopHostportManager) Remove(id string, podPortMapping *PodPortMapping) error {
+	logrus.Debug("HostPort Mapping is Disabled in CRI-O")
+	return nil
+}

internal/hostport/noop_hostport_manager_test.go

@@ -0,0 +1,19 @@
+package hostport
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// To make CodeCov happy
+func TestNoopHostportManager(t *testing.T) {
+	manager := NewNoopHostportManager()
+	assert.NotNil(t, manager)
+
+	err := manager.Add("id", nil, "")
+	assert.NoError(t, err)
+
+	err = manager.Remove("id", nil)
+	assert.NoError(t, err)
+}

pkg/config/config.go

@@ -448,6 +448,10 @@ type RuntimeConfig struct {
 	// when it is running in the host network namespace
 	// https://github.com/cri-o/cri-o/issues/5501
 	HostNetworkDisableSELinux bool `toml:"hostnetwork_disable_selinux"`
+
+	// Option to disable hostport mapping in CRI-O
+	// Default value is 'false'
+	DisableHostPortMapping bool `toml:"disable_hostport_mapping"`
 }
 
 // ImageConfig represents the "crio.image" TOML config table.
@@ -834,6 +838,7 @@ func DefaultConfig() (*Config, error) {
 			rdtConfig:                   rdt.New(),
 			ulimitsConfig:               ulimits.New(),
 			HostNetworkDisableSELinux:   true,
+			DisableHostPortMapping:      false,
 		},
 		ImageConfig: ImageConfig{
 			DefaultTransport: "docker://",

pkg/config/template.go

@@ -454,6 +454,11 @@ func initCrioTemplateConfig(c *Config) ([]*templateConfigValue, error) {
 			templateString: templateStringCrioRuntimeHostNetworkDisableSELinux,
 			group:          crioRuntimeConfig,
 			isDefaultValue: simpleEqual(dc.HostNetworkDisableSELinux, c.HostNetworkDisableSELinux),
+        },
+        {
+			templateString: templateStringCrioRuntimeDisableHostPortMapping,
+			group:          crioRuntimeConfig,
+			isDefaultValue: simpleEqual(dc.DisableHostPortMapping, c.DisableHostPortMapping),
 		},
 		{
 			templateString: templateStringCrioImageDefaultTransport,
@@ -1279,6 +1284,12 @@ const templateStringCrioRuntimeHostNetworkDisableSELinux = `# hostnetwork_disabl
 # Default value is set to true
 {{ $.Comment }}hostnetwork_disable_selinux = {{ .HostNetworkDisableSELinux }}
 
+`
+const templateStringCrioRuntimeDisableHostPortMapping = `# disable_hostport_mapping determines whether to enable/disable
+# the container hostport mapping in CRI-O.
+# Default value is set to 'false'
+{{ $.Comment }}disable_hostport_mapping = {{ .DisableHostPortMapping }}
+
 `
 
 const templateStringCrioImage = `# The crio.image table contains settings pertaining to the management of OCI images.

server/server.go

@@ -428,7 +428,13 @@ func New(
 		}
 	}
 
-	hostportManager := hostport.NewMetaHostportManager()
+	// Check for hostport mapping
+	var hostportManager hostport.HostPortManager
+	if config.RuntimeConfig.DisableHostPortMapping {
+		hostportManager = hostport.NewNoopHostportManager()
+	} else {
+		hostportManager = hostport.NewMetaHostportManager()
+	}
 
 	idMappings, err := getIDMappings(config)
 	if err != nil {

server/server_test.go

@@ -219,6 +219,14 @@ var _ = t.Describe("Server", func() {
 			mockNewServer()
 			serverConfig.StreamIdleTimeout = "200ms"
 
+			server, err := server.New(context.Background(), libMock)
+			Expect(err).To(BeNil())
+			Expect(server).ToNot(BeNil())
+		})
+		It("should succeed with hostport mapping disabled", func() {
+			mockNewServer()
+			serverConfig.RuntimeConfig.DisableHostPortMapping = true
+
 			server, err := server.New(context.Background(), libMock)
 			Expect(err).To(BeNil())
 			Expect(server).ToNot(BeNil())

test/config.bats

@@ -119,3 +119,14 @@ EOF
 	# then
 	"$CRIO_BINARY_PATH" -c "$TESTDIR"/workload.conf -d "" config
 }
+
+@test "config dir should fail with invalid disable_hostport_mapping option" {
+	# given
+	printf '[crio.runtime]\ndisable_hostport_mapping = false\n' > "$CRIO_CONFIG"
+	printf '[crio.runtime]\ndisable_hostport_mapping = "no"\n' > "$CRIO_CONFIG_DIR"/00-default
+
+	# when
+	run "$CRIO_BINARY_PATH" -c "$CRIO_CONFIG" -d "$CRIO_CONFIG_DIR"
+	# then
+	[ "$status" -ne 0 ]
+}