From 5cfda6d44ad97cf099e3dcb663e72f1d736d49aa Mon Sep 17 00:00:00 2001 From: Pedro Coutinho Date: Wed, 24 Jan 2024 20:17:10 -0800 Subject: [PATCH] Improvements to cni-plugin binaries --- cni-plugin/Makefile | 26 ++++++--- cni-plugin/cmd/calico/calico.go | 8 --- cni-plugin/cmd/install/install.go | 31 +++++++++++ cni-plugin/pkg/install/install.go | 93 +++++++++++++------------------ 4 files changed, 89 insertions(+), 69 deletions(-) create mode 100644 cni-plugin/cmd/install/install.go diff --git a/cni-plugin/Makefile b/cni-plugin/Makefile index 1244376ce16..289a1ce9acc 100644 --- a/cni-plugin/Makefile +++ b/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 \ diff --git a/cni-plugin/cmd/calico/calico.go b/cni-plugin/cmd/calico/calico.go index 055c9f26882..4b88375ffe7 100644 --- a/cni-plugin/cmd/calico/calico.go +++ b/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) } diff --git a/cni-plugin/cmd/install/install.go b/cni-plugin/cmd/install/install.go new file mode 100644 index 00000000000..f6e747004db --- /dev/null +++ b/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") + } +} diff --git a/cni-plugin/pkg/install/install.go b/cni-plugin/pkg/install/install.go index 5941ade1c88..f159d7e3fb4 100644 --- a/cni-plugin/pkg/install/install.go +++ b/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,6 +223,9 @@ 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) } @@ -231,17 +233,20 @@ func Install() error { 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.