Improvements to cni-plugin binaries

cni-plugin/Makefile

@@ -74,27 +74,40 @@ ifeq ($(ARCH),amd64)
 WINDOWS_BIN=bin/windows
 build: $(WINDOWS_BIN)/install.exe $(WINDOWS_BIN)/calico.exe $(WINDOWS_BIN)/calico-ipam.exe
 endif
-# Define go architecture flags to support arm variants
-GOARCH_FLAGS :=-e GOARCH=$(ARCH)
 
 build-all: $(addprefix sub-build-,$(VALIDARCHES))
 sub-build-%:
 	$(MAKE) build ARCH=$*
 
-## Build the Calico network plugin and ipam plugins
+## Build the Calico installation binary for the network and ipam plugins
 $(BIN)/install binary: $(SRC_FILES)
 ifeq ($(FIPS),true)
+	$(call build_cgo_boring_binary, $(PACKAGE_NAME)/cmd/install, $@)
+else
+	$(call build_binary, $(PACKAGE_NAME)/cmd/install, $@)
+endif
+
+## Build the Calico network and ipam plugins
+$(BIN)/calico: $(SRC_FILES)
+ifeq ($(FIPS), true)
 	$(call build_cgo_boring_binary, $(PACKAGE_NAME)/cmd/calico, $@)
 else
 	$(call build_binary, $(PACKAGE_NAME)/cmd/calico, $@)
 endif
 
-## Build the Calico network plugin and ipam plugins for Windows
+$(BIN)/calico-ipam: $(BIN)/calico
+	cp "$<" "$@"
+
+## Build the Calico network and ipam plugins for Windows
 $(WINDOWS_BIN)/install.exe: $(SRC_FILES)
+	mkdir -p $(WINDOWS_BIN)
+	$(call build_windows_binary, $(PACKAGE_NAME)/cmd/install, $@)
+
+$(WINDOWS_BIN)/calico.exe: $(SRC_FILES)
 	mkdir -p $(WINDOWS_BIN)
 	$(call build_windows_binary, $(PACKAGE_NAME)/cmd/calico, $@)
 
-$(WINDOWS_BIN)/calico-ipam.exe $(WINDOWS_BIN)/calico.exe: $(WINDOWS_BIN)/install.exe
+$(WINDOWS_BIN)/calico-ipam.exe: $(WINDOWS_BIN)/calico.exe
 	cp "$<" "$@"
 
 # NOTE: WINDOWS_IMAGE_REQS must be defined with the requirements to build the windows
@@ -152,9 +165,6 @@ ut: run-k8s-controller-manager $(BIN)/install $(BIN)/host-local $(BIN)/calico-ip
 	$(MAKE) ut-datastore DATASTORE_TYPE=etcdv3
 	$(MAKE) ut-datastore DATASTORE_TYPE=kubernetes
 
-$(BIN)/calico-ipam $(BIN)/calico: $(BIN)/install
-	cp "$<" "$@"
-
 ut-datastore:
 	# The tests need to run as root
 	docker run --rm -t --privileged --net=host \

cni-plugin/cmd/calico/calico.go

@@ -18,9 +18,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/sirupsen/logrus"
-
-	"github.com/projectcalico/calico/cni-plugin/pkg/install"
 	"github.com/projectcalico/calico/cni-plugin/pkg/ipamplugin"
 	"github.com/projectcalico/calico/cni-plugin/pkg/plugin"
 )
@@ -36,11 +33,6 @@ func main() {
 		plugin.Main(VERSION)
 	case "calico-ipam", "calico-ipam.exe":
 		ipamplugin.Main(VERSION)
-	case "install", "install.exe":
-		err := install.Install()
-		if err != nil {
-			logrus.WithError(err).Fatal("Error installing CNI plugin")
-		}
 	default:
 		panic("Unknown binary name: " + filename)
 	}

cni-plugin/cmd/install/install.go

@@ -0,0 +1,31 @@
+// Copyright (c) 2023 Tigera, Inc. All rights reserved.
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"github.com/sirupsen/logrus"
+
+	"github.com/projectcalico/calico/cni-plugin/pkg/install"
+)
+
+// VERSION is filled out during the build process (using git describe output)
+var VERSION string
+
+func main() {
+	err := install.Install()
+	if err != nil {
+		logrus.WithError(err).Fatal("Error installing CNI plugin")
+	}
+}

cni-plugin/pkg/install/install.go

@@ -162,34 +162,27 @@ func Install() error {
 	// Copy over any TLS assets from the SECRETS_MOUNT_DIR to the host.
 	// First check if the dir exists and has anything in it.
 	if directoryExists(c.TLSAssetsDir) {
-		logrus.Info("Installing any TLS assets")
-		mkdir(winutils.GetHostPath("/host/etc/cni/net.d/calico-tls"))
-		if err := copyFileAndPermissions(fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-ca"), winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-ca")); err != nil {
-			logrus.Warnf("Missing etcd-ca")
-		}
-		if err := copyFileAndPermissions(fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-cert"), winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-cert")); err != nil {
-			logrus.Warnf("Missing etcd-cert")
-		}
-		if err := copyFileAndPermissions(fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-key"), winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-key")); err != nil {
-			logrus.Warnf("Missing etcd-key")
+		// Only install TLS assets if at least one of them exists in the dir.
+		etcdCaPath := fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-ca")
+		etcdCertPath := fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-cert")
+		etcdKeyPath := fmt.Sprintf("%s/%s", c.TLSAssetsDir, "etcd-key")
+		if !fileExists(etcdCaPath) && !fileExists(etcdCertPath) && !fileExists(etcdKeyPath) {
+			logrus.Infof("No TLS assets found in %s, skipping", c.TLSAssetsDir)
+		} else {
+			logrus.Info("Installing any TLS assets")
+			mkdir(winutils.GetHostPath("/host/etc/cni/net.d/calico-tls"))
+			if err := copyFileAndPermissions(etcdCaPath, winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-ca")); err != nil {
+				logrus.Warnf("Missing etcd-ca")
+			}
+			if err := copyFileAndPermissions(etcdCertPath, winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-cert")); err != nil {
+				logrus.Warnf("Missing etcd-cert")
+			}
+			if err := copyFileAndPermissions(etcdKeyPath, winutils.GetHostPath("/host/etc/cni/net.d/calico-tls/etcd-key")); err != nil {
+				logrus.Warnf("Missing etcd-key")
+			}
 		}
 	}
 
-	// Set the suid bit on the binaries to allow them to run as non-root users.
-	if err := setSuidBit("/opt/cni/bin/install"); err != nil {
-		logrus.WithError(err).Fatalf("Failed to set the suid bit on the install binary")
-	}
-
-	// TODO: Remove the setSUID code here on calico and calico-ipam when they eventually
-	// get refactored to all use install as the source.
-	if err := setSuidBit("/opt/cni/bin/calico"); err != nil {
-		logrus.WithError(err).Fatalf("Failed to set the suid bit on the calico binary")
-	}
-
-	if err := setSuidBit("/opt/cni/bin/calico-ipam"); err != nil {
-		logrus.WithError(err).Fatalf("Failed to set the suid bit on the calico-ipam")
-	}
-
 	// Place the new binaries if the directory is writeable.
 	dirs := []string{winutils.GetHostPath("/host/opt/cni/bin"), winutils.GetHostPath("/host/secondary-bin-dir")}
 	binsWritten := false
@@ -198,6 +191,8 @@ func Install() error {
 			logrus.Infof("%s is not writeable, skipping", d)
 			continue
 		}
+		// Don't exec the 'calico' binary later on if it has been skipped
+		calicoBinarySkipped := true
 
 		containerBinDir := "/opt/cni/bin/"
 		// The binaries dir in the container needs to be prepended by the CONTAINER_SANDBOX_MOUNT_POINT env var on Windows Host Process Containers
@@ -213,6 +208,10 @@ func Install() error {
 		for _, binary := range files {
 			target := fmt.Sprintf("%s/%s", d, binary.Name())
 			source := fmt.Sprintf("%s/%s", containerBinDir, binary.Name())
+			// Skip the 'install' binary as it is not needed on the host
+			if binary.Name() == "install" || binary.Name() == "install.exe" {
+				continue
+			}
 			if c.skipBinary(binary.Name()) {
 				continue
 			}
@@ -224,24 +223,30 @@ func Install() error {
 				logrus.WithError(err).Errorf("Failed to install %s", target)
 				os.Exit(1)
 			}
+			if binary.Name() == "calico" || binary.Name() == "calico.exe" {
+				calicoBinarySkipped = false
+			}
 			logrus.Infof("Installed %s", target)
 		}
 
 		// Binaries were placed into at least one directory
 		logrus.Infof("Wrote Calico CNI binaries to %s\n", d)
 		binsWritten = true
 
-		// Print CNI plugin version to confirm that the binary was actually written.
-		// If this fails, it means something has gone wrong so we should retry.
-		cmd := exec.Command(d+"/calico", "-v")
-		var out bytes.Buffer
-		cmd.Stdout = &out
-		err = cmd.Run()
-		if err != nil {
-			logrus.WithError(err).Warnf("Failed getting CNI plugin version from installed binary, exiting")
-			return err
+		// Don't exec the 'calico' binary later on if it has been skipped
+		if !calicoBinarySkipped {
+			// Print CNI plugin version to confirm that the binary was actually written.
+			// If this fails, it means something has gone wrong so we should retry.
+			cmd := exec.Command(d+"/calico", "-v")
+			var out bytes.Buffer
+			cmd.Stdout = &out
+			err = cmd.Run()
+			if err != nil {
+				logrus.WithError(err).Warnf("Failed getting CNI plugin version from installed binary, exiting")
+				return err
+			}
+			logrus.Infof("CNI plugin version: %s", out.String())
 		}
-		logrus.Infof("CNI plugin version: %s", out.String())
 	}
 
 	// If binaries were not placed, exit
@@ -505,24 +510,6 @@ current-context: calico-context`
 	}
 }
 
-func setSuidBit(file string) error {
-	if runtime.GOOS == "windows" {
-		// chmod doesn't work on windows
-		logrus.Debug("chmod doesn't work on windows, skipping setSuidBit()")
-		return nil
-	}
-	fi, err := os.Stat(file)
-	if err != nil {
-		return fmt.Errorf("failed to stat file: %s", err)
-	}
-	err = os.Chmod(file, fi.Mode()|os.FileMode(uint32(8388608)))
-	if err != nil {
-		return fmt.Errorf("failed to chmod file: %s", err)
-	}
-
-	return nil
-}
-
 // destinationUptoDate compares the given files and returns
 // whether or not the destination file needs to be updated with the
 // contents of the source file.