Allow software installers with unknown versions through rather than f…

…ailing the upload (#25426) For #25201. <img width="435" alt="image" src="https://github.com/user-attachments/assets/c499902b-d461-4621-b2fc-7cb845ce71c4" /> # Checklist for submitter If some of the following don't apply, delete the relevant line.  - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated automated tests - [x] A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it) - [x] Manual QA for all new/changed functionality
fleetdm · Jan 20, 2025 · 66045db · 66045db
1 parent a3d83f4
commit 66045db
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 50 deletions.
diff --git a/changes/25201-unknown-installer-version b/changes/25201-unknown-installer-version
@@ -0,0 +1 @@
+* Revised software installer package validation to mark installers with no version as "unknown" for version rather than rejecting them
diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go
@@ -1374,13 +1374,6 @@ func (svc *Service) addMetadataToSoftwarePayload(ctx context.Context, payload *f
 		return "", ctxerr.Wrap(ctx, err, "extracting metadata from installer")
 	}
 
-	if meta.Version == "" {
-		return "", &fleet.BadRequestError{
-			Message:     fmt.Sprintf("Couldn't add. Fleet couldn't read the version from %s.", payload.Filename),
-			InternalErr: ctxerr.New(ctx, "extracting version from installer metadata"),
-		}
-	}
-
 	if len(meta.PackageIDs) == 0 {
 		return "", &fleet.BadRequestError{
 			Message:     "Couldn't add. Unable to extract necessary metadata.",

diff --git a/frontend/components/CustomLink/CustomLink.tsx b/frontend/components/CustomLink/CustomLink.tsx
@@ -12,7 +12,7 @@ interface ICustomLinkProps {
   /** Icon wraps on new line with last word */
   multiline?: boolean;
   iconColor?: Colors;
-  color?: "core-fleet-blue" | "core-fleet-black";
+  color?: "core-fleet-blue" | "core-fleet-black" | "core-fleet-white";
   /** Restricts access via keyboard when CustomLink is part of disabled UI */
   disableKeyboardNavigation?: boolean;
 }
@@ -31,6 +31,7 @@ const CustomLink = ({
 }: ICustomLinkProps): JSX.Element => {
   const customLinkClass = classnames(baseClass, className, {
     [`${baseClass}--black`]: color === "core-fleet-black",
+    [`${baseClass}--white`]: color === "core-fleet-white",
   });
 
   const target = newTab ? "_blank" : "";

diff --git a/frontend/components/CustomLink/_styles.scss b/frontend/components/CustomLink/_styles.scss
@@ -13,4 +13,12 @@
   &--black {
     color: $core-fleet-black;
   }
+
+  &--black {
+    color: $core-fleet-black;
+  }
+
+  &--white {
+    color: $core-fleet-white;
+  }
 }
diff --git a/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tsx b/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tsx
@@ -19,18 +19,6 @@ export const getErrorMessage = (err: unknown) => {
 
   if (isTimeout) {
     return "Couldn't upload. Request timeout. Please make sure your server and load balancer timeout is long enough.";
-  } else if (reason.includes("Fleet couldn't read the version from")) {
-    return (
-      <>
-        {reason}{" "}
-        <CustomLink
-          newTab
-          url={`${LEARN_MORE_ABOUT_BASE_LINK}/read-package-version`}
-          text="Learn more"
-          iconColor="core-fleet-white"
-        />
-      </>
-    );
   } else if (reason.includes("Secret variable")) {
     return generateSecretErrMsg(err);
   } else if (reason.includes("Unable to extract necessary metadata")) {
@@ -44,18 +32,6 @@ export const getErrorMessage = (err: unknown) => {
         />
       </>
     );
-  } else if (reason.includes("Fleet couldn't read the version from")) {
-    return (
-      <>
-        {reason}{" "}
-        <CustomLink
-          newTab
-          url={`${LEARN_MORE_ABOUT_BASE_LINK}/read-package-version`}
-          text="Learn more"
-          iconColor="core-fleet-white"
-        />
-      </>
-    );
   }
 
   return reason || DEFAULT_ERROR_MESSAGE;

diff --git a/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx b/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx
@@ -19,17 +19,6 @@ export const getErrorMessage = (err: unknown, software: ISoftwarePackage) => {
 
   if (isTimeout) {
     return "Couldn't upload. Request timeout. Please make sure your server and load balancer timeout is long enough.";
-  } else if (reason.includes("Fleet couldn't read the version from")) {
-    return (
-      <>
-        Couldn&apos;t edit <b>{software.name}</b>. {reason}.
-        <CustomLink
-          newTab
-          url={`${LEARN_MORE_ABOUT_BASE_LINK}/read-package-version`}
-          text="Learn more"
-        />
-      </>
-    );
   } else if (reason.includes("selected package is")) {
     return (
       <>

diff --git a/...d/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwarePackageCard/SoftwarePackageCard.tsx b/...d/pages/SoftwarePage/SoftwareTitleDetailsPage/SoftwarePackageCard/SoftwarePackageCard.tsx
@@ -30,6 +30,8 @@ import Tag from "components/Tag";
 import SoftwareIcon from "pages/SoftwarePage/components/icons/SoftwareIcon";
 import endpoints from "utilities/endpoints";
 import URL_PREFIX from "router/url_prefix";
+import { LEARN_MORE_ABOUT_BASE_LINK } from "utilities/constants";
+import CustomLink from "components/CustomLink";
 
 import DeleteSoftwareModal from "../DeleteSoftwareModal";
 import EditSoftwareModal from "../EditSoftwareModal";
@@ -228,7 +230,7 @@ const SoftwareActionsDropdown = ({
 
 interface ISoftwareInstallerCardProps {
   name: string;
-  version: string;
+  version: string | null;
   uploadedAt: string; // TODO: optional?
   status: {
     installed: number;
@@ -315,12 +317,33 @@ const SoftwareInstallerCard = ({
     );
   };
 
+  const versionInfo = version ? (
+    <span>{version}</span>
+  ) : (
+    <TooltipWrapper
+      tipContent={
+        <span>
+          Fleet couldn&apos;t read the version from ${name}.{" "}
+          <CustomLink
+            newTab
+            url={`${LEARN_MORE_ABOUT_BASE_LINK}/read-package-version`}
+            text="Learn more"
+            color="core-fleet-white"
+            iconColor="core-fleet-white"
+          />
+        </span>
+      }
+    >
+      Version (unknown)
+    </TooltipWrapper>
+  );
+
   const renderDetails = () => {
     return !uploadedAt ? (
-      <span>Version {version}</span>
+      versionInfo
     ) : (
       <>
-        <span>Version {version} &bull; </span>
+        {versionInfo} &bull;{" "}
         <TooltipWrapper
           tipContent={internationalTimeFormat(new Date(uploadedAt))}
           underline={false}

diff --git a/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.ts b/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/helpers.ts
@@ -4,7 +4,6 @@ import {
   isSoftwarePackage,
   aggregateInstallStatusCounts,
 } from "interfaces/software";
-import { DEFAULT_EMPTY_CELL_VALUE } from "utilities/constants";
 
 /**
  * Generates the data needed to render the package card. It differentiates between
@@ -30,7 +29,7 @@ export const getPackageCardInfo = (softwareTitle: ISoftwareTitleDetails) => {
     version:
       (isSoftwarePackage(packageData)
         ? packageData.version
-        : packageData.latest_version) || DEFAULT_EMPTY_CELL_VALUE,
+        : packageData.latest_version) || null,
     uploadedAt: isSoftwarePackage(packageData) ? packageData.uploaded_at : "",
     status: isSoftwarePackage(packageData)
       ? aggregateInstallStatusCounts(packageData.status)

diff --git a/frontend/styles/var/colors.scss b/frontend/styles/var/colors.scss
@@ -1,5 +1,6 @@
 // Core
 $core-fleet-black: #192147;
+$core-fleet-white: #ffffff;
 $core-fleet-blue: #3e4771;
 $core-vibrant-blue: #6a67fe;
 $core-vibrant-red: #ff5c83;

diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go
@@ -13371,7 +13371,19 @@ func (s *integrationEnterpriseTestSuite) TestPKGNoVersion() {
 		Filename:      "no_version.pkg",
 		TeamID:        &team.ID,
 	}
-	s.uploadSoftwareInstaller(t, payload, http.StatusBadRequest, "Couldn't add. Fleet couldn't read the version from no_version.pkg.")
+	s.uploadSoftwareInstaller(t, payload, http.StatusOK, "")
+
+	// title shows up with blank version
+	resp := listSoftwareTitlesResponse{}
+	s.DoJSON(
+		"GET", "/api/latest/fleet/software/titles",
+		listSoftwareTitlesRequest{},
+		http.StatusOK, &resp,
+		"team_id", fmt.Sprintf("%d", team.ID),
+	)
+	require.Len(t, resp.SoftwareTitles, 1)
+	require.Equal(t, "NoVersion.app", resp.SoftwareTitles[0].Name)
+	require.Equal(t, "", resp.SoftwareTitles[0].SoftwarePackage.Version)
 }
 
 func (s *integrationEnterpriseTestSuite) TestPKGNoBundleIdentifier() {

diff --git a/website/config/routes.js b/website/config/routes.js
@@ -826,7 +826,7 @@ module.exports.routes = {
   'GET /learn-more-about/install-scripts': '/guides/deploy-software-packages#install-script',
   'GET /learn-more-about/uninstall-scripts': '/guides/deploy-software-packages#uninstall-script',
   'GET /learn-more-about/package-metadata-extraction': '/guides/deploy-software-packages#package-metadata-extraction',
-  'GET /learn-more-about/read-package-version': '/guides/deploy-software-packages#add-a-software-package-to-a-team',
+  'GET /learn-more-about/read-package-version': '/guides/deploy-software-packages#package-metadata-extraction',
   'GET /learn-more-about/fleetctl': '/guides/fleetctl',
   'GET /feature-request': 'https://github.com/fleetdm/fleet/issues/new?assignees=&labels=~feature+fest%2C%3Aproduct&projects=&template=feature-request.md&title=',
   'GET /learn-more-about/policy-automation-run-script': '/guides/policy-automation-run-script',
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* Revised software installer package validation to mark installers with no version as "unknown" for version rather than rejecting them