[FEAT] Add new parameter disable_hostport_mapping in CRI-O

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]
@@ -217,6 +218,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

@@ -296,6 +296,9 @@ the container runtime configuration.
 **enable_pod_events**=false
 Enable CRI-O to generate the container pod-level events in order to optimize the performance of the Pod Lifecycle Event Generator (PLEG) module in Kubelet.
 
+**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

@@ -401,6 +401,9 @@ func mergeConfig(config *libconfig.Config, ctx *cli.Context) error {
 	if ctx.IsSet("enable-pod-events") {
 		config.EnablePodEvents = ctx.Bool("enable-pod-events")
 	}
+	if ctx.IsSet("disable-hostport-mapping") {
+		config.DisableHostPortMapping = ctx.Bool("disable-hostport-mapping")
+	}
 	return nil
 }
 
@@ -1114,6 +1117,12 @@ func getCrioFlags(defConf *libconfig.Config) []cli.Flag {
 			Value: defConf.IrqBalanceConfigRestoreFile,
 			Usage: "Determines if CRI-O should attempt to restore the irqbalance config at startup with the mask in this file. Use the 'disable' value to disable the restore flow entirely.",
 		},
+		&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

@@ -89,3 +89,48 @@ var _ = t.Describe("completion generation", func() {
 		),
 	)
 })
+
+// CLI Flags/Parameter test suite
+var _ = t.Describe("CLI Flags", func() {
+	const flagName = "crio"
+
+	var (
+		flagSet      *flag.FlagSet
+		ctx          *cli.Context
+		app          *cli.App
+		err          error
+		commandFlags []cli.Flag
+	)
+
+	BeforeEach(func() {
+		flagSet = flag.NewFlagSet(flagName, flag.ExitOnError)
+		app = cli.NewApp()
+		ctx = cli.NewContext(app, flagSet, nil)
+	})
+
+	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

@@ -442,6 +442,10 @@ type RuntimeConfig struct {
 
 	// namespaceManager is the internal NamespaceManager configuration
 	namespaceManager *nsmgr.NamespaceManager
+
+	// 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.
@@ -827,6 +831,7 @@ func DefaultConfig() (*Config, error) {
 			namespaceManager:            nsmgr.New(defaultNamespacesDir, ""),
 			rdtConfig:                   rdt.New(),
 			ulimitsConfig:               ulimits.New(),
+			DisableHostPortMapping:      false,
 		},
 		ImageConfig: ImageConfig{
 			DefaultTransport: "docker://",

pkg/config/template.go

@@ -450,6 +450,11 @@ func initCrioTemplateConfig(c *Config) ([]*templateConfigValue, error) {
 			group:          crioRuntimeConfig,
 			isDefaultValue: WorkloadsEqual(dc.Workloads, c.Workloads),
 		},
+		{
+			templateString: templateStringCrioRuntimeDisableHostPortMapping,
+			group:          crioRuntimeConfig,
+			isDefaultValue: simpleEqual(dc.DisableHostPortMapping, c.DisableHostPortMapping),
+		},
 		{
 			templateString: templateStringCrioImageDefaultTransport,
 			group:          crioImageConfig,
@@ -1267,6 +1272,13 @@ const templateStringCrioRuntimeWorkloads = `# The workloads table defines ways t
 {{ end }}
 `
 
+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.
 #
 # CRI-O reads its configured registries defaults from the system wide

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 ]
+}