upgrades: delete V22_2SystemPrivilegesTable upgrade and version gates

[andyyang890]

This patch deletes the V22_2SystemPrivilegesTable upgrade since its
associated unit test starts to fail when the bootstrap schema for the
system.privileges table is updated. This is safe to do since the
release engineering team has previously stated they will bump
binaryMinSupportedVersion before cutting the branch for 23.1.

Epic: None

Release note: None

[cockroach-teamcity]

This change is 

[ecwall]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andyyang890, @dt, @rafiss, and @rhu713)

---

pkg/ccl/backupccl/restore_planning.go line 1236 at r1 (raw file):

		) == nil

		if requiresRestoreSystemPrivilege && hasRestoreSystemPrivilege {

This isn't the part you changed, but this conditional seems kind of strange. I think it is easier to read as this to be more consistent with other code

if requiresRestoreSystemPrivilege {
  if err := p.CheckPrivilegeForUser(
    ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.RESTORE, p.User()
  ); err != nil {
    return pgerror.Newf(pgcode.InsufficientPrivilege,
      "only users with the admin role or the RESTORE system privilege are allowed to perform"+
      " a cluster restore")
  }
  return checkRestoreDestinationPrivileges(ctx, p, from)
}

---

pkg/ccl/backupccl/restore_planning.go line 1258 at r1 (raw file):

	}

	if hasRestoreSystemPrivilege {

hasRestoreSystemPrivilege isn't used anywhere else so return checkRestoreDestinationPrivileges(ctx, p, from) can be moved into if len(restoreStmt.Targets.Databases) > 0 { ... }.

[andyyang890]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @ecwall, @rafiss, and @rhu713)

---

pkg/ccl/backupccl/restore_planning.go line 1236 at r1 (raw file):

Previously, ecwall (Evan Wall) wrote…

This isn't the part you changed, but this conditional seems kind of strange. I think it is easier to read as this to be more consistent with other code

if requiresRestoreSystemPrivilege {
  if err := p.CheckPrivilegeForUser(
    ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.RESTORE, p.User()
  ); err != nil {
    return pgerror.Newf(pgcode.InsufficientPrivilege,
      "only users with the admin role or the RESTORE system privilege are allowed to perform"+
      " a cluster restore")
  }
  return checkRestoreDestinationPrivileges(ctx, p, from)
}

Done.

---

pkg/ccl/backupccl/restore_planning.go line 1258 at r1 (raw file):

Previously, ecwall (Evan Wall) wrote…

hasRestoreSystemPrivilege isn't used anywhere else so return checkRestoreDestinationPrivileges(ctx, p, from) can be moved into if len(restoreStmt.Targets.Databases) > 0 { ... }.

Done.

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andyyang890, @dt, @ecwall, and @rhu713)

---

pkg/ccl/backupccl/backup_planning.go line 391 at r2 (raw file):

		if requiresBackupSystemPrivilege {
			if p.CheckPrivilegeForUser(

nit: this part should be

			if err := p.CheckPrivilegeForUser(
				ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.BACKUP, p.User(),
			); err != nil {
				return pgerror.Wrapf(
					err,
					pgcode.InsufficientPrivilege,
					"only users with the admin role or the BACKUP system privilege are allowed to perform full cluster backups")
			}

---

pkg/ccl/backupccl/restore_planning.go line 1236 at r2 (raw file):

				ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.RESTORE, p.User(),
			); err != nil {
				return pgerror.Newf(pgcode.InsufficientPrivilege,

nit: use pgerror.Wrapf

---

pkg/ccl/backupccl/restore_planning.go line 1251 at r2 (raw file):

	// options.
	if len(restoreStmt.Targets.Databases) > 0 &&
		p.CheckPrivilegeForUser(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.RESTORE, p.User()) == nil {

hm, we don't need to return the error here if it fails? perhaps check with the DR team

---

pkg/sql/syntheticprivilege/README.md line 345 at r2 (raw file):

				return err
			}
			if !hasModify && !hasView {

thanks for keeping the README up to date!

[andyyang890]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @ecwall, @rafiss, and @rhu713)

---

pkg/ccl/backupccl/backup_planning.go line 391 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

nit: this part should be

			if err := p.CheckPrivilegeForUser(
				ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.BACKUP, p.User(),
			); err != nil {
				return pgerror.Wrapf(
					err,
					pgcode.InsufficientPrivilege,
					"only users with the admin role or the BACKUP system privilege are allowed to perform full cluster backups")
			}

Done.

---

pkg/ccl/backupccl/restore_planning.go line 1236 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

nit: use pgerror.Wrapf

Done.

---

pkg/ccl/backupccl/restore_planning.go line 1251 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

hm, we don't need to return the error here if it fails? perhaps check with the DR team

Ok, will do.

[andyyang890]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @ecwall, @rafiss, and @rhu713)

---

pkg/ccl/backupccl/restore_planning.go line 1251 at r2 (raw file):

Previously, andyyang890 (Andy Yang) wrote…

Ok, will do.

Talked to DR team on Slack and they're okay with filing an issue to fix that later. See #93630.

[andyyang890]

Hmmm something weird happened with the latest rebase, need to fix it before it's ready for review again

[rafiss]

thanks!

Reviewed 14 of 21 files at r1, 1 of 2 files at r3, 5 of 6 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andyyang890, @dt, @ecwall, and @rhu713)

Requested changes made

[andyyang890]

TFTR!

bors r=rafiss

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)