Merge #143286 #143287

143286: release: decrease log level for preflight check r=celiala a=rail

Previously, the preflight check was logging at the trace level, which created a huge log file, what caused the API server to reject submissions due to the log file size. At the same time, the catalog still showed the image as published, which was misleading. This commit reduce the log level to error.

Release note: none
Epic: none


143287: sql: avoid dangling role reference in default privileges r=rafiss a=rafiss

fixes #142777
Release note (bug fix): Fixed a bug that could leave behind a dangling reference to a dropped role if that role had default privileges granted to itself. The bug was caused by defining privileges such as: `ALTER DEFAULT PRIVILEGES FOR ROLE self_referencing_role GRANT INSERT ON TABLES TO self_referencing_role`

Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

Author: 3 people

build/teamcity/internal/cockroach/release/publish/publish-redhat-release.sh

@@ -77,7 +77,7 @@ mkdir -p artifacts
 docker run \
   --rm \
   --security-opt=label=disable \
-  --env PFLT_LOGLEVEL=trace \
+  --env PFLT_LOGLEVEL=error \
   --env PFLT_ARTIFACTS=/artifacts \
   --env PFLT_LOGFILE=/artifacts/preflight.log \
   --env PFLT_CERTIFICATION_PROJECT_ID="$rhel_project_id" \

pkg/sql/catalog/catprivilege/default_privilege.go

@@ -118,6 +118,7 @@ func (d *Mutable) GrantDefaultPrivileges(
 			return err
 		}
 	}
+	d.removeRoleIfEmptyPrivileges(role, defaultPrivilegesForRole)
 	return nil
 }
 
@@ -135,14 +136,20 @@ func (d *Mutable) RevokeDefaultPrivileges(
 			return err
 		}
 	}
+	d.removeRoleIfEmptyPrivileges(role, defaultPrivilegesForRole)
+	return nil
+}
 
+// removeRoleIfEmptyPrivilegesc checks if there are any default privileges for
+// the given role remaining on the descriptor. If there are no privileges left
+// remaining and the descriptor is in the default state, the role is removed.
+func (d *Mutable) removeRoleIfEmptyPrivileges(
+	role catpb.DefaultPrivilegesRole, defaultPrivilegesForRole *catpb.DefaultPrivilegesForRole,
+) {
 	defaultPrivilegesPerObject := defaultPrivilegesForRole.DefaultPrivilegesPerObject
-	// Check if there are any default privileges remaining on the descriptor.
-	// If there are no privileges left remaining and the descriptor is in the
-	// default state, we can remove it.
 	for _, defaultPrivs := range defaultPrivilegesPerObject {
 		if len(defaultPrivs.Users) != 0 {
-			return nil
+			return
 		}
 	}
 
@@ -155,12 +162,11 @@ func (d *Mutable) RevokeDefaultPrivileges(
 			!GetRoleHasAllPrivilegesOnTargetObject(defaultPrivilegesForRole, privilege.Schemas) ||
 			!GetPublicHasUsageOnTypes(defaultPrivilegesForRole) ||
 			!GetPublicHasExecuteOnFunctions(defaultPrivilegesForRole)) {
-		return nil
+		return
 	}
 
 	// There no entries remaining, remove the entry for the role.
 	d.defaultPrivilegeDescriptor.RemoveUser(role)
-	return nil
 }
 
 // CreatePrivilegesFromDefaultPrivileges creates privileges for a

pkg/sql/logictest/testdata/logic_test/drop_role_with_default_privileges

@@ -183,3 +183,23 @@ statement ok
 DROP USER IF EXISTS a_user_that_does_not_exist;
 
 subtest end
+
+# Verify that a granting default privileges to the same role the defaults are
+# being defined for has no undesirable side effect.
+subtest default_priv_granted_to_self
+
+statement ok
+CREATE ROLE self_referencing_role
+
+statement ok
+ALTER DEFAULT PRIVILEGES FOR ROLE self_referencing_role GRANT INSERT ON TABLES TO self_referencing_role
+
+statement ok
+DROP ROLE self_referencing_role
+
+query I
+SELECT count(*) FROM crdb_internal.invalid_objects
+----
+0
+
+subtest end