Bug 1715001: when extracting tools from payload, 'oc version' should …

…report payload version

pkg/cli/admin/release/extract_tools.go

@@ -150,27 +150,30 @@ func (o *ExtractOptions) extractCommand(command string) error {
 			Command: "oc",
 			Mapping: extract.Mapping{Image: "cli-artifacts", From: "usr/share/openshift/mac/oc"},
 
-			LinkTo:        []string{"kubectl"},
-			Readme:        readmeCLIUnix,
-			ArchiveFormat: "openshift-client-mac-%s.tar.gz",
+			LinkTo:             []string{"kubectl"},
+			Readme:             readmeCLIUnix,
+			InjectReleaseImage: true,
+			ArchiveFormat:      "openshift-client-mac-%s.tar.gz",
 		},
 		{
 			OS:      "linux",
 			Command: "oc",
 			Mapping: extract.Mapping{Image: "cli", From: "usr/bin/oc"},
 
-			LinkTo:        []string{"kubectl"},
-			Readme:        readmeCLIUnix,
-			ArchiveFormat: "openshift-client-linux-%s.tar.gz",
+			LinkTo:             []string{"kubectl"},
+			Readme:             readmeCLIUnix,
+			InjectReleaseImage: true,
+			ArchiveFormat:      "openshift-client-linux-%s.tar.gz",
 		},
 		{
 			OS:      "windows",
 			Command: "oc",
 			Mapping: extract.Mapping{Image: "cli-artifacts", From: "usr/share/openshift/windows/oc.exe"},
 
-			Readme:        readmeCLIWindows,
-			ArchiveFormat: "openshift-client-windows-%s.zip",
-			AsZip:         true,
+			Readme:             readmeCLIWindows,
+			InjectReleaseImage: true,
+			ArchiveFormat:      "openshift-client-windows-%s.zip",
+			AsZip:              true,
 		},
 		{
 			OS:      "darwin",
@@ -489,10 +492,19 @@ func (o *ExtractOptions) extractCommand(command string) error {
 
 		// copy the input to disk
 		if target.InjectReleaseImage {
+			releaseInfo := exactReleaseImage
 			var matched bool
-			matched, err = copyAndReplaceReleaseImage(w, r, 4*1024, exactReleaseImage)
+			errmsg := ""
+			switch target.Command {
+			case "oc":
+				releaseInfo = releaseName
+				errmsg = fmt.Sprintf("warning: Unable to inject release image info into %s, could not determine version", target.Command)
+			default:
+				errmsg = fmt.Sprintf("warning: Unable to inject release image info into %s, installer will not be locked to the correct image", target.Command)
+			}
+			matched, err = copyAndReplaceReleaseInfo(w, r, 4*1024, releaseInfo)
 			if !matched {
-				fmt.Fprintf(o.ErrOut, "warning: Unable to replace release image location into %s, installer will not be locked to the correct image\n", target.TargetName)
+				fmt.Fprintf(o.ErrOut, "%s\n", errmsg)
 			}
 		} else {
 			_, err = io.Copy(w, r)
@@ -612,23 +624,23 @@ func (o *ExtractOptions) extractCommand(command string) error {
 }
 
 const (
-	// installerReplacement is the location within the installer binary that we can insert our
-	// release payload string
-	installerReplacement = "\x00_RELEASE_IMAGE_LOCATION_\x00XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\x00"
+	// binaryReplacement is the location within the installer binary that we can insert our
+	// release image info string
+	binaryReplacement = "\x00_RELEASE_IMAGE_LOCATION_\x00XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\x00"
 )
 
-// copyAndReplaceReleaseImage performs a targeted replacement for binaries that contain a special marker string
-// as a constant, replacing the marker with releaseImage and a NUL terminating byte. It returns true if the
+// copyAndReplaceReleaseInfo performs a targeted replacement for binaries that contain a special marker string
+// as a constant, replacing the marker with releaseInfo and a NUL terminating byte. It returns true if the
 // replacement was performed.
-func copyAndReplaceReleaseImage(w io.Writer, r io.Reader, bufferSize int, releaseImage string) (bool, error) {
-	if len(releaseImage)+1 > len(installerReplacement) {
-		return false, fmt.Errorf("the release image pull spec is longer than the maximum replacement length for the installer binary")
+func copyAndReplaceReleaseInfo(w io.Writer, r io.Reader, bufferSize int, releaseInfo string) (bool, error) {
+	if len(releaseInfo)+1 > len(binaryReplacement) {
+		return false, fmt.Errorf("the release image pull spec is longer than the maximum replacement length for the binary")
 	}
-	if bufferSize < len(installerReplacement) {
-		return false, fmt.Errorf("the buffer size must be greater than %d bytes", len(installerReplacement))
+	if bufferSize < len(binaryReplacement) {
+		return false, fmt.Errorf("the buffer size must be greater than %d bytes", len(binaryReplacement))
 	}
 
-	match := []byte(installerReplacement[:len(releaseImage)+1])
+	match := []byte(binaryReplacement[:len(releaseInfo)+1])
 	offset := 0
 	max := bufferSize
 	buf := make([]byte, max+offset)
@@ -644,15 +656,15 @@ func copyAndReplaceReleaseImage(w io.Writer, r io.Reader, bufferSize int, releas
 			if index != -1 {
 				klog.V(2).Infof("Found match at %d (len=%d, offset=%d, n=%d)", index, len(buf), offset, n)
 				// the replacement starts at the beginning of the match, contains the release string and a terminating NUL byte
-				copy(buf[index:index+len(releaseImage)], []byte(releaseImage))
-				buf[index+len(releaseImage)] = 0x00
+				copy(buf[index:index+len(releaseInfo)], []byte(releaseInfo))
+				buf[index+len(releaseInfo)] = 0x00
 				matched = true
 			}
 		}
 
 		// write everything that we have already searched (excluding the end of the buffer that will
 		// be checked next pass)
-		nextOffset := end - len(installerReplacement)
+		nextOffset := end - len(binaryReplacement)
 		if nextOffset < 0 || matched {
 			nextOffset = 0
 		}

pkg/cli/admin/release/extract_tools_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func Test_copyAndReplaceReleaseImage(t *testing.T) {
-	baseLen := len(installerReplacement)
+	baseLen := len(binaryReplacement)
 	tests := []struct {
 		name         string
 		r            *bytes.Buffer
@@ -61,7 +61,7 @@ func Test_copyAndReplaceReleaseImage(t *testing.T) {
 			original := make([]byte, len(src))
 			copy(original, src)
 
-			got, err := copyAndReplaceReleaseImage(w, tt.r, tt.buffer, tt.releaseImage)
+			got, err := copyAndReplaceReleaseInfo(w, tt.r, tt.buffer, tt.releaseImage)
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("copyAndReplaceReleaseImage() error = %v, wantErr %v", err, tt.wantErr)
 			}
@@ -89,7 +89,7 @@ func fakeInput(lengths ...int) *bytes.Buffer {
 	buf := &bytes.Buffer{}
 	for _, l := range lengths {
 		if l == 0 {
-			buf.WriteString(installerReplacement)
+			buf.WriteString(binaryReplacement)
 		} else {
 			b := byte(rand.Intn(256))
 			buf.Write(bytes.Repeat([]byte{b}, l))

pkg/cli/version/version.go

@@ -26,7 +26,8 @@ import (
 
 type Version struct {
 	kversion.Version
-	OpenShiftVersion string `json:"openshiftVersion,omitempty"`
+	ReleaseClientVersion string `json:"releaseClientVersion,omitempty"`
+	OpenShiftVersion     string `json:"openshiftVersion,omitempty"`
 }
 
 var (
@@ -108,7 +109,13 @@ func (o *VersionOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 // Run is copied from upstream version command, with added OpenShift server version logic
 func (o *VersionOptions) Run() error {
 	var versionInfo Version
-	clientVersion := version.Get()
+	clientVersion, reportedVersion, err := version.ExtractVersion()
+	if err != nil {
+		return err
+	}
+	if len(reportedVersion) != 0 {
+		versionInfo.ReleaseClientVersion = reportedVersion
+	}
 	versionInfo.ClientVersion = &clientVersion
 
 	var serverErr error
@@ -145,7 +152,11 @@ func (o *VersionOptions) Run() error {
 	switch o.Output {
 	case "":
 		if versionInfo.ClientVersion != nil {
-			fmt.Fprintf(o.Out, "Client Version: %s\n", clientVersion.GitVersion)
+			if len(versionInfo.ReleaseClientVersion) != 0 {
+				fmt.Fprintf(o.Out, "Client Version: %s\n", reportedVersion)
+			} else {
+				fmt.Fprintf(o.Out, "Client Version: %s\n", clientVersion.GitVersion)
+			}
 		}
 		if len(versionInfo.OpenShiftVersion) != 0 {
 			fmt.Fprintf(o.Out, "Server Version: %s\n", fmt.Sprintf("%s", versionInfo.OpenShiftVersion))

pkg/version/version.go

@@ -19,6 +19,7 @@ package version
 import (
 	"fmt"
 	"runtime"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/version"
 )
@@ -55,3 +56,47 @@ func Get() version.Info {
 		Platform:     fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH),
 	}
 }
+
+// This file handles correctly identifying the default release version, which is expected to be
+// replaced in the binary post-compile by the release name extracted from a payload. The expected modification is:
+//
+// 1. Extract a release binary that contains a cli image
+// 2. Identify the release name, add a NUL terminator byte (0x00) to the end, calculate length
+// 3. Length must be less than 300 bytes
+// 4. Search through the cli binary looking for `\x00_RELEASE_IMAGE_LOCATION_\x00<PADDING_TO_LENGTH>`
+//    where padding is the ASCII character X and length is the total length of the image
+// 5. Overwrite that chunk of the bytes if found, otherwise return error.
+
+var (
+	// defaultReleaseInfoPadded may be replaced in the binary with a pull spec that overrides defaultReleaseInfo as
+	// a null-terminated string within the allowed character length. This allows a distributor to override the payload
+	// location without having to rebuild the source.
+	defaultReleaseInfoPadded = "\x00_RELEASE_IMAGE_LOCATION_\x00XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\x00"
+	defaultReleaseInfoPrefix = "\x00_RELEASE_IMAGE_LOCATION_\x00"
+	defaultReleaseInfoLength = len(defaultReleaseInfoPadded)
+)
+
+// ExtractVersion() abstracts how the binary loads the default release payload. We want to lock the binary
+// to the pull spec of the payload we test it with, and since a payload contains the cli image we can't
+// know that at build time. Instead, we make it possible to replace the release string after build via a
+// known constant in the binary.  When extracting oc from release image, 'oc version' reports the payload version.
+func ExtractVersion() (version.Info, string, error) {
+	if strings.HasPrefix(defaultReleaseInfoPadded, defaultReleaseInfoPrefix) {
+		return Get(), "", nil
+	}
+	nullTerminator := strings.IndexByte(defaultReleaseInfoPadded, '\x00')
+	if nullTerminator == -1 {
+		// the binary has been altered, but we didn't find a null terminator within the release name constant which is an error
+		return version.Info{}, "", fmt.Errorf("release name location was replaced but without a null terminator before %d bytes", defaultReleaseInfoLength)
+	}
+	if nullTerminator > len(defaultReleaseInfoPadded) {
+		// the binary has been altered, but the null terminator is *longer* than the constant encoded in the binary
+		return version.Info{}, "", fmt.Errorf("release name location contains no null-terminator and constant is corrupted")
+	}
+	releaseName := defaultReleaseInfoPadded[:nullTerminator]
+	if len(releaseName) == 0 {
+		// the binary has been altered, but the replaced release name is empty which is incorrect
+		return version.Info{}, "", fmt.Errorf("release name location is empty, this binary was incorrectly generated")
+	}
+	return Get(), releaseName, nil
+}