[PLAT-18725] Universe ownership validation skip if universe is incompatible with the check

asc-073-yb · asc-073-yb · commit 855fd217ff66 · 2025-10-11T09:02:25.000Z
Summary: Diff: https://phorge.dev.yugabyte.com/D44905 introduced a universe owner check related to attach/detach flow. Adding some guardrails around it: - Old universes - non-YSQL universes (since we can't query the yba consistency info on them) - Also will skip this validation if the runtime config `yb.attach_detach.enabled` is not enabled. Test Plan: Manual testing that universe actions are working for old universes and non-YSQL universes Updated Unit tests in UtilTest, UniverseActionsControllerTest, UniverseControllerTest to accomodate new change + more scenarios coverage Reviewers: anijhawan, nsingh Reviewed By: anijhawan Differential Revision: https://phorge.dev.yugabyte.com/D47350
diff --git a/managed/src/main/java/com/yugabyte/yw/common/Util.java b/managed/src/main/java/com/yugabyte/yw/common/Util.java
@@ -19,6 +19,7 @@
 import com.yugabyte.yw.cloud.PublicCloudConstants.OsType;
 import com.yugabyte.yw.commissioner.Common;
 import com.yugabyte.yw.commissioner.Common.CloudType;
+import com.yugabyte.yw.common.config.GlobalConfKeys;
 import com.yugabyte.yw.common.config.RuntimeConfGetter;
 import com.yugabyte.yw.common.gflags.GFlagsUtil;
 import com.yugabyte.yw.common.logging.LogUtil;
@@ -1200,6 +1201,41 @@ public static void validateUniverseOwnershipAndNotDetached(
       ConfigHelper configHelper,
       YsqlQueryExecutor ysqlQueryExecutor,
       RuntimeConfGetter confGetter) {
+    // Skip ownership validation if attach/detach feature is not enabled
+    boolean attachDetachEnabled = confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled);
+    if (!attachDetachEnabled) {
+      log.debug(
+          "Skipping ownership validation for universe {} (attach/detach feature not enabled)",
+          universe.getName());
+      return;
+    }
+
+    UniverseDefinitionTaskParams.Cluster primaryCluster =
+        universe.getUniverseDetails().getPrimaryCluster();
+    if (primaryCluster == null || primaryCluster.userIntent == null) {
+      log.warn(
+          "Skipping ownership validation for universe {} due to missing primary cluster or user"
+              + " intent",
+          universe.getName());
+      return;
+    }
+
+    UniverseDefinitionTaskParams.UserIntent primaryIntent = primaryCluster.userIntent;
+
+    // Skip ownership validation for older DB versions that don't support attach/detach
+    String dbVersion = primaryIntent.ybSoftwareVersion;
+    if (dbVersion == null
+        || compareYBVersions(
+                dbVersion, "2024.2.0.0-b1", "2.23.1.0-b29", true /* suppressFormatError */)
+            < 0) {
+      log.debug(
+          "Skipping ownership validation for universe {} with DB version {} (older than minimum"
+              + " required for attach/detach)",
+          universe.getName(),
+          dbVersion);
+      return;
+    }
+
     if (universe.getUniverseDetails().universeDetached) {
       throw new PlatformServiceException(
           BAD_REQUEST,
@@ -1232,13 +1268,21 @@ public static boolean isUniverseOwner(
 
     UUID storedYwUuid = getStoredYwUuid(universe, ysqlQueryExecutor, confGetter);
 
+    // If we cannot retrieve ownership info (e.g., YCQL-only universe), skip validation
+    if (storedYwUuid == null) {
+      log.debug(
+          "Cannot determine ownership for universe {} (YSQL not available), assuming owned",
+          universe.getName());
+      return true;
+    }
+
     // If stored YW UUID is DETACHED_UNIVERSE_UUID, the universe is orphaned and operations should
     // not be allowed
-    if (storedYwUuid == AttachDetachSpec.DETACHED_UNIVERSE_UUID) {
+    if (storedYwUuid.equals(AttachDetachSpec.DETACHED_UNIVERSE_UUID)) {
       throw new PlatformServiceException(
           BAD_REQUEST,
           String.format(
-              "Universe %s has no owner. Operations are not allowed on orphaned" + " universes.",
+              "Universe %s has no owner. Operations are not allowed on orphaned universes.",
               universe.getName()));
     }
     return currentYwUuid.equals(storedYwUuid);
@@ -1249,6 +1293,13 @@ public static UUID getStoredYwUuid(
     try {
       YsqlQueryExecutor.ConsistencyInfoResp consistencyInfo =
           ysqlQueryExecutor.getConsistencyInfo(universe);
+      // Return null if consistency info could not be retrieved (e.g., YCQL-only universe)
+      if (consistencyInfo == null) {
+        log.debug(
+            "Could not retrieve consistency info for universe {} (likely YSQL is not enabled)",
+            universe.getName());
+        return null;
+      }
       return consistencyInfo.getYwUUID();
     } catch (Exception e) {
       log.error("Failed to query YW UUID for universe {}: {}", universe.getName(), e);
diff --git a/managed/src/main/java/com/yugabyte/yw/models/AttachDetachSpec.java b/managed/src/main/java/com/yugabyte/yw/models/AttachDetachSpec.java
@@ -783,11 +783,12 @@ private void updateYwUuidInConsistencyTable(
         return;
       }
 
-      if (Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter) != DETACHED_UNIVERSE_UUID) {
+      UUID storedYwUuid = Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter);
+      if (storedYwUuid != null && !storedYwUuid.equals(DETACHED_UNIVERSE_UUID)) {
         throw new PlatformServiceException(
             BAD_REQUEST,
             "Universe already has an owner "
-                + Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter)
+                + storedYwUuid
                 + ", please delete universe metadata from this YBA"
                 + " instance.");
       }
diff --git a/managed/src/test/java/com/yugabyte/yw/common/UtilTest.java b/managed/src/test/java/com/yugabyte/yw/common/UtilTest.java
@@ -20,12 +20,14 @@
 import com.cronutils.utils.StringUtils;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.ImmutableMap;
+import com.yugabyte.yw.common.config.GlobalConfKeys;
 import com.yugabyte.yw.common.config.RuntimeConfGetter;
 import com.yugabyte.yw.common.utils.FileUtils;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.Cluster;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.ClusterType;
 import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.UserIntent;
+import com.yugabyte.yw.models.AttachDetachSpec;
 import com.yugabyte.yw.models.Customer;
 import com.yugabyte.yw.models.Universe;
 import com.yugabyte.yw.models.Users;
@@ -607,8 +609,9 @@ public void testIsUniverseOwner_NullStoredYwUuid() {
     when(consistencyInfo.getYwUUID()).thenReturn(null);
     when(ysqlQueryExecutor.getConsistencyInfo(any(Universe.class))).thenReturn(consistencyInfo);
 
-    assertFalse(
-        "Should return false when stored YW UUID is null (not matching current UUID)",
+    // When stored UUID is null (YCQL-only universe), assume owned and return true
+    assertTrue(
+        "Should return true when stored YW UUID is null (YCQL-only universe, skip validation)",
         Util.isUniverseOwner(universe, configHelper, ysqlQueryExecutor, confGetter));
   }
 
@@ -638,15 +641,9 @@ public void testGetStoredYwUuid_NullConsistencyInfo() {
     when(universe.getName()).thenReturn("test-universe");
     when(ysqlQueryExecutor.getConsistencyInfo(any(Universe.class))).thenReturn(null);
 
-    PlatformServiceException exception =
-        assertThrows(
-            "Should throw PlatformServiceException when consistency info is null",
-            PlatformServiceException.class,
-            () -> Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter));
-
-    assertTrue(
-        "Exception should mention error querying YW UUID",
-        exception.getMessage().contains("Error querying YW UUID"));
+    // When consistency info is null (e.g., YCQL-only universe), should return null gracefully
+    UUID result = Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter);
+    assertNull("Should return null when consistency info is null (YCQL-only universe)", result);
   }
 
   @Test
@@ -677,8 +674,18 @@ public void testvalidateUniverseOwnershipAndNotDetached_DetachedUniverse() {
     YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
     RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
 
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
     UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
     taskParams.universeDetached = true;
+
+    // Set up primary cluster with valid version to ensure validation runs
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2024.2.0.0-b1"; // Valid version for attach/detach
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
     when(universe.getUniverseDetails()).thenReturn(taskParams);
     when(universe.getName()).thenReturn("test-universe");
 
@@ -702,9 +709,20 @@ public void testvalidateUniverseOwnershipAndNotDetached_NotOwner() {
     YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
     RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
 
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
     UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
     taskParams.universeDetached = false;
+
+    // Set up primary cluster with valid version to ensure validation runs
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2024.2.0.0-b1"; // Valid version for attach/detach
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
     when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
 
     UUID currentYwUuid = UUID.randomUUID();
     UUID storedYwUuid = UUID.randomUUID(); // Different UUID
@@ -736,9 +754,20 @@ public void testvalidateUniverseOwnershipAndNotDetached_ValidOwner() {
     YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
     RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
 
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
     UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
     taskParams.universeDetached = false;
+
+    // Set up primary cluster with valid version to ensure validation runs
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2024.2.0.0-b1"; // Valid version for attach/detach
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
     when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
 
     UUID currentYwUuid = UUID.randomUUID();
     UUID storedYwUuid = currentYwUuid; // Same UUID
@@ -754,4 +783,145 @@ public void testvalidateUniverseOwnershipAndNotDetached_ValidOwner() {
     Util.validateUniverseOwnershipAndNotDetached(
         universe, configHelper, ysqlQueryExecutor, confGetter);
   }
+
+  @Test
+  public void testIsUniverseOwner_DetachedUniverseUuid() {
+    Universe universe = mock(Universe.class);
+    ConfigHelper configHelper = mock(ConfigHelper.class);
+    YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
+    RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
+
+    UUID currentYwUuid = UUID.randomUUID();
+    when(configHelper.getYugawareUUID()).thenReturn(currentYwUuid);
+    when(universe.getName()).thenReturn("test-universe");
+
+    // Stored UUID is DETACHED_UNIVERSE_UUID (orphaned universe)
+    YsqlQueryExecutor.ConsistencyInfoResp consistencyInfo =
+        mock(YsqlQueryExecutor.ConsistencyInfoResp.class);
+    when(consistencyInfo.getYwUUID()).thenReturn(AttachDetachSpec.DETACHED_UNIVERSE_UUID);
+    when(ysqlQueryExecutor.getConsistencyInfo(any(Universe.class))).thenReturn(consistencyInfo);
+
+    PlatformServiceException exception =
+        assertThrows(
+            "Should throw exception for orphaned universe (DETACHED_UNIVERSE_UUID)",
+            PlatformServiceException.class,
+            () -> Util.isUniverseOwner(universe, configHelper, ysqlQueryExecutor, confGetter));
+
+    assertTrue(
+        "Exception should mention orphaned universe",
+        exception.getMessage().contains("has no owner")
+            && exception.getMessage().contains("orphaned"));
+  }
+
+  @Test
+  public void testvalidateUniverseOwnershipAndNotDetached_OldDbVersion() {
+    Universe universe = mock(Universe.class);
+    ConfigHelper configHelper = mock(ConfigHelper.class);
+    YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
+    RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
+
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
+    UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
+    taskParams.universeDetached = false;
+
+    // Set up universe with old DB version (before attach/detach support)
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2.18.0.0-b1"; // Old version
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
+    when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
+
+    // Should not throw - validation should be skipped for old versions
+    Util.validateUniverseOwnershipAndNotDetached(
+        universe, configHelper, ysqlQueryExecutor, confGetter);
+  }
+
+  @Test
+  public void testvalidateUniverseOwnershipAndNotDetached_NullDbVersion() {
+    Universe universe = mock(Universe.class);
+    ConfigHelper configHelper = mock(ConfigHelper.class);
+    YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
+    RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
+
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
+    UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
+    taskParams.universeDetached = false;
+
+    // Set up universe with null DB version
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = null; // Null version
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
+    when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
+
+    // Should not throw - validation should be skipped for null versions
+    Util.validateUniverseOwnershipAndNotDetached(
+        universe, configHelper, ysqlQueryExecutor, confGetter);
+  }
+
+  @Test
+  public void testvalidateUniverseOwnershipAndNotDetached_YcqlOnlyUniverse() {
+    Universe universe = mock(Universe.class);
+    ConfigHelper configHelper = mock(ConfigHelper.class);
+    YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
+    RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
+
+    // Mock attach/detach feature enabled
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(true);
+
+    UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
+    taskParams.universeDetached = false;
+
+    // Set up YCQL-only universe (YSQL disabled)
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2024.2.0.0-b1"; // Valid version
+    userIntent.enableYSQL = false; // YCQL-only
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
+    when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
+
+    // Mock configHelper to return a valid UUID (needed if validation progresses to isUniverseOwner)
+    when(configHelper.getYugawareUUID()).thenReturn(UUID.randomUUID());
+
+    // Should not throw - validation should be skipped for YCQL-only universes
+    Util.validateUniverseOwnershipAndNotDetached(
+        universe, configHelper, ysqlQueryExecutor, confGetter);
+  }
+
+  @Test
+  public void testvalidateUniverseOwnershipAndNotDetached_FeatureDisabled() {
+    Universe universe = mock(Universe.class);
+    ConfigHelper configHelper = mock(ConfigHelper.class);
+    YsqlQueryExecutor ysqlQueryExecutor = mock(YsqlQueryExecutor.class);
+    RuntimeConfGetter confGetter = mock(RuntimeConfGetter.class);
+
+    // Mock attach/detach feature DISABLED
+    when(confGetter.getGlobalConf(GlobalConfKeys.attachDetachEnabled)).thenReturn(false);
+
+    UniverseDefinitionTaskParams taskParams = new UniverseDefinitionTaskParams();
+    taskParams.universeDetached = true;
+
+    // Set up universe with valid version
+    UniverseDefinitionTaskParams.UserIntent userIntent =
+        new UniverseDefinitionTaskParams.UserIntent();
+    userIntent.ybSoftwareVersion = "2024.2.0.0-b1";
+    taskParams.upsertPrimaryCluster(userIntent, null);
+
+    when(universe.getUniverseDetails()).thenReturn(taskParams);
+    when(universe.getName()).thenReturn("test-universe");
+
+    // validation should be skipped entirely when feature is disabled
+    Util.validateUniverseOwnershipAndNotDetached(
+        universe, configHelper, ysqlQueryExecutor, confGetter);
+  }
 }
diff --git a/managed/src/test/java/com/yugabyte/yw/controllers/UniverseActionsControllerTest.java b/managed/src/test/java/com/yugabyte/yw/controllers/UniverseActionsControllerTest.java
@@ -188,6 +188,7 @@ public void testUniversePauseValidUUID() {
           UniverseDefinitionTaskParams.UserIntent userIntent =
               new UniverseDefinitionTaskParams.UserIntent();
           userIntent.providerType = Common.CloudType.aws;
+          userIntent.ybSoftwareVersion = "2.20.0.0-b1";
           universeDetails.upsertPrimaryCluster(userIntent, null);
           universe.setUniverseDetails(universeDetails);
         };
diff --git a/managed/src/test/java/com/yugabyte/yw/controllers/UniverseControllerTest.java b/managed/src/test/java/com/yugabyte/yw/controllers/UniverseControllerTest.java
@@ -201,6 +201,7 @@ public void testUniverseDestroyValidUUID() {
           UniverseDefinitionTaskParams.UserIntent userIntent =
               new UniverseDefinitionTaskParams.UserIntent();
           userIntent.providerType = Common.CloudType.aws;
+          userIntent.ybSoftwareVersion = "2.20.0.0-b1";
           universeDetails.upsertPrimaryCluster(userIntent, null);
           universe.setUniverseDetails(universeDetails);
         };
@@ -252,6 +253,7 @@ public void testUniverseDestroyValidUUIDIsForceDelete() {
           UniverseDefinitionTaskParams.UserIntent userIntent =
               new UniverseDefinitionTaskParams.UserIntent();
           userIntent.providerType = Common.CloudType.aws;
+          userIntent.ybSoftwareVersion = "2.20.0.0-b1";
           universeDetails.upsertPrimaryCluster(userIntent, null);
           universe.setUniverseDetails(universeDetails);
         };
@@ -312,6 +314,7 @@ public void testUniverseDestroyValidUUIDIsForceDeleteAndDeleteBackup(
           UniverseDefinitionTaskParams.UserIntent userIntent =
               new UniverseDefinitionTaskParams.UserIntent();
           userIntent.providerType = Common.CloudType.aws;
+          userIntent.ybSoftwareVersion = "2.20.0.0-b1";
           universeDetails.upsertPrimaryCluster(userIntent, null);
           universe.setUniverseDetails(universeDetails);
         };

Original file line number	Diff line number	Diff line change
`@@ -783,11 +783,12 @@ private void updateYwUuidInConsistencyTable(`
`783`	`783`	`return;`
`784`	`784`	`}`
`785`	`785`
`786`		`- if (Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter) != DETACHED_UNIVERSE_UUID) {`
	`786`	`+ UUID storedYwUuid = Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter);`
	`787`	`+ if (storedYwUuid != null && !storedYwUuid.equals(DETACHED_UNIVERSE_UUID)) {`
`787`	`788`	`throw new PlatformServiceException(`
`788`	`789`	`BAD_REQUEST,`
`789`	`790`	`"Universe already has an owner "`
`790`		`- + Util.getStoredYwUuid(universe, ysqlQueryExecutor, confGetter)`
	`791`	`+ + storedYwUuid`
`791`	`792`	`+ ", please delete universe metadata from this YBA"`
`792`	`793`	`+ " instance.");`
`793`	`794`	`}`