[release-1.11] NETOBSERV-2598: NETOBSERV-2517: Remove dependency on oc/kubectl (#461)

* Remove dependency on oc/kubectl

* Prevent future path traversal

The --filename argument of the go binary, which isn't used at the
moment, is not sanitized for path traversal. This could be an unnoticed
vulnerability if we chose to leverage it later on.

This change uses go 1.24 "os.Root" API to prevent path traversal.

* Use go-client to delete daemonset

* fix options array

* fix namespace not propagated to go runtime

* separate foreground and background cases

* Add "eval" on command exec

For some reason (??) it worked for get-flows but not for get-metrics ...
"eval" makes it work for metrics too

---------

Co-authored-by: Joel Takvorian <jtakvori@redhat.com>
Co-authored-by: Julien Pinsonneau <jpinsonn@redhat.com>

Author: 3 people

Dockerfile

@@ -10,6 +10,7 @@ ARG LDFLAGS
 WORKDIR /opt/app-root
 
 COPY cmd cmd
+COPY internal internal
 COPY main.go main.go
 COPY go.mod go.mod
 COPY go.sum go.sum
@@ -25,12 +26,6 @@ COPY scripts/ scripts/
 COPY Makefile Makefile
 COPY .mk/ .mk/
 
-# Install oc to allow collector to run commands
-RUN set -x; \
-    OC_TAR_URL="https://mirror.openshift.com/pub/openshift-v4/$(uname -m)/clients/ocp/latest/openshift-client-linux.tar.gz" && \
-    curl -L -q -o /tmp/oc.tar.gz "$OC_TAR_URL" && \
-    tar -C /tmp -xvf /tmp/oc.tar.gz oc kubectl
-
 # Embed commands in case users want to pull it from collector image
 RUN USER=netobserv VERSION=main make oc-commands
 
@@ -42,8 +37,6 @@ FROM --platform=linux/$TARGETARCH registry.access.redhat.com/ubi9/ubi:9.7-177023
 WORKDIR /
 
 COPY --from=builder /opt/app-root/build .
-COPY --from=builder /tmp/oc /usr/bin/oc
-COPY --from=builder /tmp/kubectl /usr/bin/kubectl
 COPY --from=builder --chown=65532:65532 /opt/app-root/output /output
 USER 65532:65532
 

Dockerfile.downstream

@@ -1,9 +1,6 @@
 ARG BUILDVERSION
 ARG BUILDVERSION_Y
 
-# Make kubectl & oc scripts available for copy
-FROM registry.redhat.io/openshift4/ose-cli-rhel9:v4.18.0-202502040032.p0.ga50d4c0.assembly.stream.el9 as ose-cli
-
 # Build the manager binary
 FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:v1.25 as builder
 ARG BUILDVERSION
@@ -13,6 +10,7 @@ ARG AGENT_IMAGE=registry.redhat.io/network-observability/network-observability-e
 WORKDIR /opt/app-root
 
 COPY cmd cmd
+COPY internal internal
 COPY main.go main.go
 COPY go.mod go.mod
 COPY go.sum go.sum
@@ -46,8 +44,6 @@ COPY --from=builder /opt/app-root/build .
 COPY --from=builder --chown=65532:65532 /opt/app-root/output /output
 COPY LICENSE /licenses/
 COPY README.downstream ./README
-COPY --from=ose-cli /usr/bin/kubectl /usr/bin/kubectl
-COPY --from=ose-cli /usr/bin/oc /usr/bin/oc
 
 USER 65532:65532
 

cmd/flow_capture.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"encoding/json"
-	"os"
 	"strings"
 	"time"
 
@@ -44,21 +43,13 @@ func startFlowCollector() {
 			":", "") // get rid of offensive colons
 	}
 
-	var f *os.File
-	err := os.MkdirAll("./output/flow/", 0700)
+	// Create a text file to receive json chunks; the file will be fixed and renamed as json later, when pulled in shell.
+	f, err := createOutputFile("flow", filename+".txt")
 	if err != nil {
-		log.Errorf("Create directory failed: %v", err.Error())
-		log.Fatal(err)
-	}
-	log.Debug("Created flow folder")
-
-	f, err = os.Create("./output/flow/" + filename + ".txt")
-	if err != nil {
-		log.Errorf("Create file %s failed: %v", filename, err.Error())
-		log.Fatal(err)
+		log.Fatalf("Creating output file failed: %v", err)
 	}
 	defer f.Close()
-	log.Debug("Created flow logs txt file")
+	log.Debugf("Created flow logs txt file: %s", f.Name())
 
 	// Initialize sqlite DB
 	db := initFlowDB(filename)
@@ -126,7 +117,7 @@ func startFlowCollector() {
 		// terminate capture if max time reached
 		now := currentTime()
 		duration := now.Sub(startupTime)
-		if int(duration) > int(maxTime) {
+		if duration > maxTime {
 			if exit := onLimitReached(); exit {
 				log.Infof("Capture reached %s, exiting now...", maxTime)
 				return

cmd/flow_db.go

@@ -4,7 +4,6 @@ import (
 	"database/sql"
 	"encoding/json"
 	"fmt"
-	"os"
 
 	"github.com/netobserv/flowlogs-pipeline/pkg/config"
 	// need to import the sqlite3 driver
@@ -13,16 +12,15 @@ import (
 
 func initFlowDB(filename string) *sql.DB {
 	// SQLite is a file based database.
-	flowsDB := "./output/flow/" + filename + ".db"
-
 	log.Println("Creating database...")
-	file, err := os.Create(flowsDB) // Create SQLite file
+	f, err := createOutputFile("flow", filename+".db")
 	if err != nil {
-		log.Errorf("Failed to create flows db file: %v", err.Error())
-		log.Fatal(err)
+		log.Fatalf("Creating output db file failed: %v", err)
 	}
-	file.Close()
-	log.Println("flows.db created")
+	flowsDB := f.Name()
+	f.Close()
+
+	log.Println("flows db created")
 	// Open SQLite database
 	db, err := sql.Open("sqlite3", flowsDB)
 	if err != nil {

cmd/packet_capture.go

@@ -4,7 +4,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"os"
 	"sort"
 	"strings"
 	"time"
@@ -55,14 +54,7 @@ func startPacketCollector() {
 			":", "") // get rid of offensive colons
 	}
 
-	err := os.MkdirAll("./output/pcap/", 0700)
-	if err != nil {
-		log.Error("Create directory failed", err)
-		return
-	}
-	log.Debug("Created pcap folder")
-
-	f, err := os.Create("./output/pcap/" + filename + ".pcapng")
+	f, err := createOutputFile("pcap", filename+".pcapng")
 	if err != nil {
 		log.Fatal(err)
 	}

cmd/root.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -10,6 +11,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/netobserv/network-observability-cli/internal/pkg/kubernetes"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -23,13 +25,14 @@ const (
 )
 
 var (
-	log      = logrus.New()
-	logLevel string
-	port     int
-	filename string
-	options  string
-	maxTime  time.Duration
-	maxBytes int64
+	log       = logrus.New()
+	logLevel  string
+	port      int
+	filename  string
+	namespace string
+	options   string
+	maxTime   time.Duration
+	maxBytes  int64
 
 	currentTime = time.Now
 	startupTime = currentTime()
@@ -69,6 +72,7 @@ func init() {
 	rootCmd.PersistentFlags().StringVarP(&options, "options", "", "", "Options(s)")
 	rootCmd.PersistentFlags().DurationVarP(&maxTime, "maxtime", "", 5*time.Minute, "Maximum capture time")
 	rootCmd.PersistentFlags().Int64VarP(&maxBytes, "maxbytes", "", 50000000, "Maximum capture bytes")
+	rootCmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", "netobserv-cli", "Namespace where agent pods are running")
 	rootCmd.PersistentFlags().BoolVarP(&useMocks, "mock", "", false, "Use mock")
 
 	c := make(chan os.Signal, 1)
@@ -148,11 +152,10 @@ func onLimitReached() bool {
 			app.Stop()
 		}
 		if isBackground {
-			out, err := exec.Command("/oc-netobserv", "stop").Output()
+			err := kubernetes.DeleteDaemonSet(context.Background(), namespace)
 			if err != nil {
-				log.Fatal(err)
+				log.Error(err)
 			}
-			fmt.Printf("%s", out)
 			fmt.Print(`Thank you for using...`)
 			printBanner()
 
@@ -187,3 +190,17 @@ func onLimitReached() bool {
 
 	return shouldExit
 }
+
+// Create output file, preventing path traversal
+func createOutputFile(kind, filename string) (*os.File, error) {
+	base := "./output/" + kind + "/"
+	if err := os.MkdirAll(base, 0700); err != nil {
+		return nil, err
+	}
+	root, err := os.OpenRoot(base)
+	if err != nil {
+		return nil, err
+	}
+	defer root.Close()
+	return root.Create(filename)
+}

commands/netobserv

@@ -189,24 +189,43 @@ trap onExit EXIT
 setup
 
 if [[ "$command" == "flows" || "$command" == "packets" || "$command" == "metrics" ]]; then
-  # convert options to string
-  optionStr="${options//--/}"
-  optionStr="${optionStr// /|}"
+  # convert options array to pipe-separated string
+  optionStr=""
+  for opt in "${options[@]}"; do
+    opt="${opt#--}"  # remove leading --
+    if [ -n "$optionStr" ]; then
+      optionStr="$optionStr|$opt"
+    else
+      optionStr="$opt"
+    fi
+  done
 
   # prepare commands & args
   runCommand="sleep infinity"
-  execCommand="/network-observability-cli get-$command${optionStr:+" --options" "${optionStr}"} --loglevel $logLevel --maxtime $maxTime"
-  if [[ "$command" == "flows" || "$command" == "packets" ]]; then
-    execCommand="$execCommand --maxbytes $maxBytes"
-  else 
-    copy="false"
-  fi
-  
   if [[ "$runBackground" == "true" || "$outputYAML" == "true" ]]; then
+    # For background mode: wrap in bash -c with proper escaping for pod command
+    execCommand="/network-observability-cli get-$command${optionStr:+" --options \\\"${optionStr}\\\""} --loglevel $logLevel --maxtime $maxTime --namespace $namespace"
+    if [[ "$command" == "flows" || "$command" == "packets" ]]; then
+      execCommand="$execCommand --maxbytes $maxBytes"
+    fi
     runCommand="bash -c \"$execCommand && $runCommand\""
     execCommand=""
+  else
+    # For foreground mode: will be used in kubectl exec
+    # Build as array to avoid quoting issues
+    execCommandBase="/network-observability-cli get-$command"
+    execCommandArgs="--loglevel $logLevel --maxtime $maxTime --namespace $namespace"
+    if [[ "$command" == "flows" || "$command" == "packets" ]]; then
+      execCommandArgs="$execCommandArgs --maxbytes $maxBytes"
+    fi
+    if [ -n "$optionStr" ]; then
+      # Store options for later use
+      execOptions="$optionStr"
+    else
+      execOptions=""
+    fi
   fi
-
+  
   # override extra configs
   overrides="'{\"spec\":{\"serviceAccount\": \"netobserv-cli\"}}'"
 
@@ -251,13 +270,13 @@ if [[ "$command" == "flows" || "$command" == "packets" || "$command" == "metrics
 
   captureStarted=true
 
-  if [ -n "${execCommand}" ]; then
+  if [[ "$runBackground" != "true" && "$outputYAML" != "true" ]]; then
     echo "Executing collector command... "
-    cmd="${K8S_CLI_BIN} exec -i --tty \
-      -n $namespace \
-      collector \
-      -- $execCommand"
-    eval "$cmd"
+    if [ -n "$execOptions" ]; then
+      eval "${K8S_CLI_BIN} exec -i --tty -n $namespace collector -- $execCommandBase --options \"$execOptions\" $execCommandArgs"
+    else
+      eval "${K8S_CLI_BIN} exec -i --tty -n $namespace collector -- $execCommandBase $execCommandArgs"
+    fi
   elif [[ "$command" == "flows" || "$command" == "packets" ]]; then
     echo "Background capture started. Use:"
     echo " - '${K8S_CLI_BIN} netobserv follow' to see the capture progress"

go.mod

@@ -19,12 +19,6 @@ require (
 	sigs.k8s.io/e2e-framework v0.6.0
 )
 
-require (
-	github.com/golang-jwt/jwt/v5 v5.3.0 // indirect
-	golang.org/x/mod v0.30.0 // indirect
-	golang.org/x/sync v0.19.0 // indirect
-)
-
 require (
 	github.com/Masterminds/semver/v3 v3.4.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
@@ -41,6 +35,7 @@ require (
 	github.com/go-openapi/jsonreference v0.21.0 // indirect
 	github.com/go-openapi/swag v0.23.1 // indirect
 	github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
+	github.com/golang-jwt/jwt/v5 v5.3.0 // indirect
 	github.com/google/gnostic-models v0.7.0 // indirect
 	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/pprof v0.0.0-20250423184734-337e5dd93bb4 // indirect
@@ -79,8 +74,10 @@ require (
 	go.opentelemetry.io/otel/trace v1.39.0 // indirect
 	go.yaml.in/yaml/v2 v2.4.3 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
+	golang.org/x/mod v0.30.0 // indirect
 	golang.org/x/net v0.48.0 // indirect
 	golang.org/x/oauth2 v0.34.0 // indirect
+	golang.org/x/sync v0.19.0 // indirect
 	golang.org/x/sys v0.40.0 // indirect
 	golang.org/x/term v0.38.0 // indirect
 	golang.org/x/text v0.32.0 // indirect

internal/pkg/kubernetes/kubernetes.go

@@ -0,0 +1,24 @@
+package kubernetes
+
+import (
+	"context"
+	"fmt"
+
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+)
+
+func DeleteDaemonSet(ctx context.Context, namespace string) error {
+	config, err := rest.InClusterConfig()
+	if err != nil {
+		return fmt.Errorf("cannot get Kubernetes InClusterConfig: %w", err)
+	}
+
+	clientset, err := kubernetes.NewForConfig(config)
+	if err != nil {
+		return fmt.Errorf("cannot create Kubernetes client from InClusterConfig: %w", err)
+	}
+
+	return clientset.AppsV1().DaemonSets(namespace).Delete(ctx, "netobserv-cli", v1.DeleteOptions{})
+}

scripts/functions.sh

@@ -386,7 +386,7 @@ function follow() {
 }
 
 function copyOutput() {
-  echo "Copying collector output files..."
+  echo "Copying collector files to ${OUTPUT_PATH}..."
   if [[ ! -d ${OUTPUT_PATH} ]]; then
     mkdir -p ${OUTPUT_PATH} >/dev/null
   fi
@@ -1108,7 +1108,7 @@ function check_args_and_apply() {
         logLevel=$value
         filter=${filter/$key=$logLevel/}
       else
-        echo "invalid value for --action"
+        echo "invalid value for --log-level"
       fi
       ;;
     *max-time) # Max time