[BACKPORT 2024.1][PLAT-13324] We should not have multiple versions wi…

…th matching release tags

Summary:
Original commit: e016db2 / D33932
Making a release version unique except when a tag is present.

To support this for internal machines - especially portals, we also have to
be able to generate a tag from the file name. This is done by using any characters
that are in place of "-b123" in the file name.

Add a constraint unique(version, release_tag) on the release table to support this.
But, in PG14 we cannot make null 'unique' (its in pg15). Instead, the Release model
will encode a null tag to a constant to store in the db, and decode it back to null
for any clients.

Fixed some bugs with the migration process, especially around checking for helm charts.

Test Plan:
1. do not allow multiple releases of the same version without release tags
2. Migration of older "matched" versions works correctly (older = not parsing version_metadata.json)
3. Migration of newer "matched" versions works correctly (parses version_metadata.json)
4. Tag generation works correctly
5. Migration from ReleaseMetadata works correctly
6. null encoding for release tags works correctly

Reviewers: muthu, #yba-api-review, anijhawan

Reviewed By: muthu, #yba-api-review, anijhawan

Subscribers: yugaware, sanketh

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D33963

managed/src/main/java/com/yugabyte/yw/common/ExtraMigrationManager.java

@@ -93,7 +93,9 @@ public void R__Release_Metadata_Migration() {
 
   private void createArtifacts(ReleaseMetadata metadata, Release release) {
     // create helm artifact if necessary
-    if (metadata.chartPath != null && release.getKubernetesArtifact() == null) {
+    if (metadata.chartPath != null
+        && !metadata.chartPath.isEmpty()
+        && release.getKubernetesArtifact() == null) {
       ReleaseLocalFile rlf = ReleaseLocalFile.create(metadata.chartPath);
       release.addArtifact(
           ReleaseArtifact.create(

managed/src/main/java/com/yugabyte/yw/common/ReleaseManager.java

@@ -643,17 +643,21 @@ public synchronized void importLocalReleases() {
               .forEach(
                   p -> {
                     ReleasesUtils.ExtractedMetadata metadata = null;
-                    releasesUtils.metadataFromPath(p);
                     try {
+                      log.debug("checking local file {}", p.toString());
                       metadata = releasesUtils.metadataFromPath(p);
+                      String rawVersion = metadata.version.split("-")[0];
                       if (metadata.platform == ReleaseArtifact.Platform.KUBERNETES) {
-                        localReleaseNameValidation(metadata.version, null, p.toString());
+                        localReleaseNameValidation(rawVersion, null, p.toString());
                       } else {
-                        localReleaseNameValidation(metadata.version, p.toString(), null);
+                        localReleaseNameValidation(rawVersion, p.toString(), null);
                       }
                     } catch (RuntimeException e) {
                       log.error(
-                          "local release " + p.getFileName().toString() + " failed validation");
+                          "local release "
+                              + p.getFileName().toString()
+                              + " failed validation: "
+                              + e.getLocalizedMessage());
                       // continue forEach
                       return;
                     }
@@ -664,13 +668,15 @@ public synchronized void importLocalReleases() {
                             metadata.platform,
                             metadata.architecture,
                             rlf.getFileUUID());
-                    Release release = Release.getByVersion(metadata.version);
+                    Release release = Release.getByVersion(metadata.version, metadata.releaseTag);
                     if (release == null) {
-                      release = Release.create(metadata.version, metadata.release_type);
+                      release =
+                          Release.create(
+                              metadata.version, metadata.release_type, metadata.releaseTag);
                     }
                     release.addArtifact(artifact);
                   });
-        } catch (IOException e) {
+        } catch (Exception e) {
           log.error("failed to read local releases", e);
         }
 
@@ -1152,7 +1158,10 @@ public Map<String, ReleaseContainer> getAllReleaseContainersByVersion() {
       return Release.getAll().stream()
           .collect(
               Collectors.toMap(
-                  release -> release.getVersion(),
+                  release ->
+                      release.getReleaseTag() == null
+                          ? release.getVersion()
+                          : String.format("%s-%s", release.getVersion(), release.getReleaseTag()),
                   release -> releaseContainerFactory.newReleaseContainer(release)));
     } else {
       return getReleaseMetadata().entrySet().stream()

managed/src/main/java/com/yugabyte/yw/common/ReleasesUtils.java

@@ -10,6 +10,7 @@
 import com.typesafe.config.Config;
 import com.yugabyte.yw.cloud.PublicCloudConstants.Architecture;
 import com.yugabyte.yw.common.ReleaseManager.ReleaseMetadata;
+import com.yugabyte.yw.common.utils.FileUtils;
 import com.yugabyte.yw.models.Release;
 import com.yugabyte.yw.models.ReleaseArtifact;
 import com.yugabyte.yw.models.ReleaseLocalFile;
@@ -52,10 +53,14 @@ public class ReleasesUtils {
   public final String YB_PACKAGE_REGEX =
       "yugabyte-(?:ee-)?(.*)-(alma|centos|linux|el8|darwin)(.*).tar.gz";
   // We can see helm packages as either yugabyte-<version>.tgz or yugabyte-version-helm.tgz
-  public final String YB_HELM_PACKAGE_REGEX = "yugabyte-(?:ee-)?(.*?)(?:-helm)?.(?:tar.gz|tgz)";
+  public final String YB_HELM_PACKAGE_REGEX =
+      "yugabyte-(?:ee-)?(?:(?:(.*?)(?:-helm))|(\\d+\\.\\d+\\.\\d+)).(?:tar.gz|tgz)";
   // Match release form 2.16.1.2 and return 2.16 or 2024.1.0.0 and return 2024
   public final String YB_VERSION_TYPE_REGEX = "(2\\.\\d+|\\d\\d\\d\\d)";
 
+  public final String YB_TAG_REGEX =
+      "yugabyte-(?:ee-)?(?:.*)-(?!b\\d+)(.*)-(?:alma|centos|linux|el8|darwin)(?:.*).tar.gz";
+
   // The first version where YBDB must include a metadata.json file
   public final String YBDB_METADATA_REQUIRED_VERSION = "2024.1.0.0";
 
@@ -79,6 +84,7 @@ public MetadataParseException(String errMsg) {
 
   public static class ExtractedMetadata {
     public String version;
+    public String releaseTag;
     public Release.YbType yb_type;
     public String sha256;
     public ReleaseArtifact.Platform platform;
@@ -105,6 +111,7 @@ public ExtractedMetadata metadataFromPath(Path releaseFilePath) {
           versionMetadataFromInputStream(
               new BufferedInputStream(new FileInputStream(releaseFilePath.toFile())));
       em.sha256 = sha256;
+      em.releaseTag = tagFromName(releaseFilePath.toString());
       return em;
     } catch (MetadataParseException e) {
       // Fallback to file name validation
@@ -128,6 +135,7 @@ public ExtractedMetadata versionMetadataFromURL(URL url) {
       // Fallback to file name validation
       log.warn("falling back to file name metadata parsing for url " + url.toString(), e);
       ExtractedMetadata em = metadataFromName(url.getFile());
+      em.releaseTag = tagFromName(url.toString());
       return em;
     } catch (IOException e) {
       log.error("failed to open url " + url.toString());
@@ -144,7 +152,7 @@ private ExtractedMetadata versionMetadataFromInputStream(InputStream inputStream
       TarArchiveEntry entry;
       while ((entry = tarInput.getNextEntry()) != null) {
         if (entry.getName().endsWith("version_metadata.json")) {
-          log.debug("found version_metadata.json");
+          log.trace("found version_metadata.json");
           // We can reasonably assume that the version metadata json is small enough to read in
           // oneshot
           byte[] fileContent = new byte[(int) entry.getSize()];
@@ -217,7 +225,7 @@ private ExtractedMetadata versionMetadataFromInputStream(InputStream inputStream
           return metadata;
         }
       } // end of while loop
-      log.error("No verison_metadata found in given input stream");
+      log.error("No version_metadata found in given input stream");
       throw new MetadataParseException("no version_metadata found");
     } catch (java.io.IOException e) {
       log.error("failed reading the local file", e);
@@ -249,7 +257,7 @@ public void cleanupUntracked() {
         UUID dirUUID = UUID.fromString(uploadedDir.getName());
         if (ReleaseLocalFile.get(dirUUID) == null) {
           log.debug("deleting untracked local file " + dirUUID);
-          if (!uploadedDir.delete()) {
+          if (!FileUtils.deleteDirectory(uploadedDir)) {
             log.error("failed to delete " + uploadedDir.getAbsolutePath());
           }
           continue;
@@ -318,7 +326,7 @@ private ExtractedMetadata metadataFromHelmChart(InputStream helmStream) {
       TarArchiveEntry entry;
       while ((entry = tarInput.getNextEntry()) != null) {
         if (entry.getName().endsWith("Chart.yaml") || entry.getName().endsWith("Chart.yml")) {
-          log.debug("Found Chart.yml");
+          log.trace("Found Chart.yml");
           // We can reasonably assume that the version metadata json is small enough to read in
           // oneshot
           byte[] fileContent = new byte[(int) entry.getSize()];
@@ -439,4 +447,13 @@ private String ybaCurrentVersion() {
     }
     return (String) versionCfg.get("version");
   }
+
+  private String tagFromName(String name) {
+    Pattern tagPattern = Pattern.compile(YB_TAG_REGEX);
+    Matcher tagMatch = tagPattern.matcher(name);
+    if (tagMatch.find()) {
+      return tagMatch.group(1);
+    }
+    return null;
+  }
 }

managed/src/main/java/com/yugabyte/yw/controllers/ReleaseController.java

@@ -233,7 +233,10 @@ public Result list(UUID customerUUID, Boolean includeMetadata, @Nullable String
                   })
               .collect(
                   Collectors.toMap(
-                      entry -> entry.getVersion(),
+                      entry ->
+                          entry.getReleaseTag() == null
+                              ? entry.getVersion()
+                              : String.format("%s-%s", entry.getVersion(), entry.getReleaseTag()),
                       entry -> releasesUtils.releaseToReleaseMetadata(entry)));
     } else {
       Map<String, Object> releases = releaseManager.getReleaseMetadata();

managed/src/main/java/com/yugabyte/yw/controllers/ReleasesController.java

@@ -75,6 +75,7 @@ public Result create(UUID customerUUID, Http.Request request) {
     Customer.getOrBadRequest(customerUUID);
     CreateRelease reqRelease =
         formFactory.getFormDataOrBadRequest(request.body().asJson(), CreateRelease.class);
+    // Validate the version/tag combo doesn't exist
     if (reqRelease.release_uuid == null) {
       log.trace("generating random release UUID as one was not provided");
       reqRelease.release_uuid = UUID.randomUUID();

managed/src/main/java/com/yugabyte/yw/models/Release.java

@@ -2,6 +2,7 @@
 
 import static play.mvc.Http.Status.BAD_REQUEST;
 
+import autovalue.shaded.com.google.common.annotations.VisibleForTesting;
 import com.yugabyte.yw.cloud.PublicCloudConstants.Architecture;
 import com.yugabyte.yw.common.PlatformServiceException;
 import com.yugabyte.yw.controllers.apiModels.CreateRelease;
@@ -30,6 +31,9 @@
 @Setter
 @Slf4j
 public class Release extends Model {
+  // on pg14, we can't make a null value unique, so instead use this hard-coded constant
+  public static final String NULL_CONSTANT = "NULL-VALUE-DO-NOT-USE-AS-INPUT";
+
   @Id private UUID releaseUUID;
 
   @Column(nullable = false)
@@ -70,10 +74,25 @@ public static Release create(String version, String releaseType) {
     return create(UUID.randomUUID(), version, releaseType);
   }
 
+  public static Release create(String version, String releaseType, String releaseTag) {
+    return create(UUID.randomUUID(), version, releaseType, releaseTag);
+  }
+
   public static Release create(UUID releaseUUID, String version, String releaseType) {
+    return create(releaseUUID, version, releaseType, null);
+  }
+
+  public static Release create(
+      UUID releaseUUID, String version, String releaseType, String releaseTag) {
+    if (Release.getByVersion(version, releaseTag) != null) {
+      throw new PlatformServiceException(
+          BAD_REQUEST,
+          String.format("release version %s with tag %s already exists", version, releaseTag));
+    }
     Release release = new Release();
     release.releaseUUID = releaseUUID;
     release.version = version;
+    release.releaseTag = encodeReleaseTag(releaseTag);
     release.releaseType = releaseType;
     release.yb_type = YbType.YBDB;
     release.state = ReleaseState.INCOMPLETE;
@@ -89,14 +108,26 @@ public static Release createFromRequest(CreateRelease reqRelease) {
     } else {
       release.releaseUUID = UUID.randomUUID();
     }
+    // Validate the release doesn't already exist - either by uuid or verison/tag combo
+    if (Release.get(release.releaseUUID) != null) {
+      throw new PlatformServiceException(
+          BAD_REQUEST, "release with uuid " + release.releaseUUID + " already exists");
+    }
+    if (Release.getByVersion(reqRelease.version, reqRelease.release_tag) != null) {
+      throw new PlatformServiceException(
+          BAD_REQUEST,
+          String.format(
+              "release version %s with tag %s already exists",
+              reqRelease.version, reqRelease.release_tag));
+    }
 
     // Required fields
     release.version = reqRelease.version;
     release.releaseType = reqRelease.release_type;
     release.yb_type = YbType.valueOf(reqRelease.yb_type);
 
     // Optional fields
-    release.releaseTag = reqRelease.release_tag;
+    release.releaseTag = encodeReleaseTag(reqRelease.release_tag);
     if (reqRelease.release_date != null) {
       try {
         release.releaseDate = Date.from(Instant.ofEpochSecond(reqRelease.release_date));
@@ -138,8 +169,12 @@ public static Release getOrBadRequest(UUID releaseUUID) {
   }
 
   public static Release getByVersion(String version) {
-    // TODO: Need to map between version and tag.
-    return find.query().where().eq("version", version).findOne();
+    return Release.getByVersion(version, null);
+  }
+
+  public static Release getByVersion(String version, String tag) {
+    tag = encodeReleaseTag(tag);
+    return find.query().where().eq("version", version).eq("release_tag", tag).findOne();
   }
 
   public void addArtifact(ReleaseArtifact artifact) {
@@ -176,10 +211,20 @@ public ReleaseArtifact getKubernetesArtifact() {
   }
 
   public void setReleaseTag(String tag) {
-    this.releaseTag = tag;
+    this.releaseTag = encodeReleaseTag(tag);
     save();
   }
 
+  public String getReleaseTag() {
+    return decodeReleaseTag(this.releaseTag);
+  }
+
+  // Mainly used for test validation, please use `getReleaseTag()` outside of unit tests.
+  @VisibleForTesting
+  public String getRawReleaseTag() {
+    return releaseTag;
+  }
+
   public void setReleaseDate(Date date) {
     this.releaseDate = date;
     save();
@@ -237,4 +282,21 @@ public Set<Universe> getUniverses() {
     String formattedVersion = this.version;
     return Universe.universeDetailsIfReleaseExists(formattedVersion);
   }
+
+  private static String encodeReleaseTag(String releaseTag) {
+    if (releaseTag == null) {
+      return NULL_CONSTANT;
+    } else if (releaseTag.equals(NULL_CONSTANT)) {
+      throw new PlatformServiceException(
+          BAD_REQUEST, "cannot set release tag to " + Release.NULL_CONSTANT);
+    }
+    return releaseTag;
+  }
+
+  private static String decodeReleaseTag(String releaseTag) {
+    if (releaseTag == null || releaseTag.equals(NULL_CONSTANT)) {
+      return null;
+    }
+    return releaseTag;
+  }
 }

managed/src/main/java/db/migration/default_/common/R__Release_Metadata_Migration.java

@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import io.ebean.DB;
+import io.ebean.SqlRow;
 import jakarta.persistence.PersistenceException;
 import java.sql.Connection;
 import java.sql.SQLException;
@@ -25,10 +26,10 @@ public Integer getChecksum() {
     int codeChecksum = 82918231; // Change me if you want to force migration to run
     MurmurHash3 murmurHash3 = new MurmurHash3();
     try {
-      String jsonStr =
+      SqlRow row =
           DB.sqlQuery("SELECT value FROM yugaware_property WHERE name='SoftwareReleases'")
-              .findOne()
-              .getString("value");
+              .findOne();
+      String jsonStr = row == null ? "" : row.getString("value");
       return murmurHash3.stringHash(jsonStr, codeChecksum);
     } catch (PersistenceException e) {
       log.warn("failed to query yugaware property: " + e.getLocalizedMessage());

managed/src/main/resources/db/migration/default_/postgres/V341__Release_Version_Tag_Unique.sql

@@ -0,0 +1,6 @@
+
+UPDATE release 
+  SET release_tag='NULL-VALUE-DO-NOT-USE-AS-INPUT'
+  WHERE release_tag is null;
+ALTER TABLE release
+  ADD CONSTRAINT version_tag_unique UNIQUE (version, release_tag);

managed/src/test/java/com/yugabyte/yw/models/ReleasesTest.java

@@ -1,6 +1,8 @@
 package com.yugabyte.yw.models;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
 
 import com.yugabyte.yw.cloud.PublicCloudConstants;
 import com.yugabyte.yw.cloud.PublicCloudConstants.Architecture;
@@ -21,6 +23,12 @@ public void testCreateAndGet() {
     assertEquals(foundRelease.getReleaseUUID(), createdRelease.getReleaseUUID());
     assertEquals(foundRelease.getVersion(), createdRelease.getVersion());
     assertEquals(foundRelease.getReleaseType(), createdRelease.getReleaseType());
+
+    assertThrows(
+        PlatformServiceException.class, () -> Release.create(UUID.randomUUID(), "1.2.3", "LTS"));
+    Release release2 = Release.create(UUID.randomUUID(), "1.2.3", "LTS", "tag1");
+    Release foundRelease2 = Release.get(release2.getReleaseUUID());
+    assertEquals("tag1", foundRelease2.getReleaseTag());
   }
 
   @Test(expected = PlatformServiceException.class)
@@ -97,4 +105,13 @@ public void testAddingK8S() {
     release.addArtifact(art1);
     assertEquals(Release.ReleaseState.ACTIVE, release.getState());
   }
+
+  @Test
+  public void testTagRenaming() {
+    Release release = Release.create("1.2.3.4", "LTS");
+    assertNull(release.getReleaseTag());
+    assertEquals(Release.NULL_CONSTANT, release.getRawReleaseTag());
+    assertThrows(
+        PlatformServiceException.class, () -> release.setReleaseTag(Release.NULL_CONSTANT));
+  }
 }