Repository: helm/helm (go)

Issue:
Helm works only with owner role with psql
Hi, we are using helm with the sql storage (psql) instead classic secrets storage. We are trying to separate permissions for database. But helm only works with access role = owner, but not with read-write rights. Looks like helm always runs migrations with `CREATE TABLE`, which are not allowed for read-write user. Is there a way to fix it in helm, maybe by manual checking existence of table or migration?

<img width="1409" alt="image" src="https://user-images.githubusercontent.com/55360517/230487954-30417ea2-c9c9-48b1-876c-bf275e0129dc.png">



Output of `helm version`:
version.BuildInfo{Version:"v3.10.3", GitCommit:"835b7334cfe2e5e27870ab3ed4135f136eecc704", GitTreeState:"clean", GoVersion:"go1.19.4"}

Output of `kubectl version`:
k version -oyaml
clientVersion:
  buildDate: "2022-10-12T10:47:25Z"
  compiler: gc
  gitCommit: 434bfd82814af038ad94d62ebe59b133fcb50506
  gitTreeState: clean
  gitVersion: v1.25.3
  goVersion: go1.19.2
  major: "1"
  minor: "25"
  platform: darwin/arm64
kustomizeVersion: v4.5.7
serverVersion:
  buildDate: "2022-03-16T14:02:06Z"
  compiler: gc
  gitCommit: 38d3c1f3d5306401bcf39a71bad3b5a5106033d7
  gitTreeState: clean
  gitVersion: v1.21.11
  goVersion: go1.16.15
  major: "1"
  minor: "21"
  platform: linux/amd64

Cloud Provider/Platform (AKS, GKE, Minikube etc.): 
Vanila self managed kubernetes



Pull request:
sql driver with rw-roles
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:
Docs tells us that we need owner role only for first run to setup db. But in reality we always need owner role, because we always trying to execute "create table if not exists", what forbidden for read-write and less privileged users. (closes #11970)
https://helm.sh/docs/topics/permissions_sql_storage_backend/#initialization

In this pr we first check if migrations not applied, and only after run migrations.

Also it allows to apply custom migrations, in our case without RLS and GRANT ALL.

**Special notes for your reviewer**:
It's my first pr, sorry, if something wrong.
Can we finish pr due Thursday? 


**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


Patch:
diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go
index 18f51f3fd..892f10bbb 100644
--- a/pkg/storage/driver/sql.go
+++ b/pkg/storage/driver/sql.go
@@ -94,8 +94,42 @@ func (s *SQL) Name() string {
 	return SQLDriverName
 }
 
+// Check if all migrations al
+func (s *SQL) checkAlreadyApplied(migrations []*migrate.Migration) bool {
+	// make map (set) of ids for fast search
+	migrationsIds := make(map[string]struct{})
+	for _, migration := range migrations {
+		migrationsIds[migration.Id] = struct{}{}
+	}
+
+	// get list of applied migrations
+	migrate.SetDisableCreateTable(true)
+	records, err := migrate.GetMigrationRecords(s.db.DB, postgreSQLDialect)
+	migrate.SetDisableCreateTable(false)
+	if err != nil {
+		s.Log("checkAlreadyApplied: failed to get migration records: %v", err)
+		return false
+	}
+
+	for _, record := range records {
+		if _, ok := migrationsIds[record.Id]; ok {
+			s.Log("checkAlreadyApplied: found previous migration (Id: %v) applied at %v", record.Id, record.AppliedAt)
+			delete(migrationsIds, record.Id)
+		}
+	}
+
+	// check if all migrations appliyed
+	if len(migrationsIds) != 0 {
+		for id := range migrationsIds {
+			s.Log("checkAlreadyApplied: find unapplied migration (id: %v)", id)
+		}
+		return false
+	}
+	return true
+}
+
 func (s *SQL) ensureDBSetup() error {
-	// Populate the database with the relations we need if they don't exist yet
+
 	migrations := &migrate.MemoryMigrationSource{
 		Migrations: []*migrate.Migration{
 			{
@@ -121,9 +155,9 @@ func (s *SQL) ensureDBSetup() error {
 						CREATE INDEX ON %s (%s);
 						CREATE INDEX ON %s (%s);
 						CREATE INDEX ON %s (%s);
-						
+	
 						GRANT ALL ON %s TO PUBLIC;
-
+	
 						ALTER TABLE %s ENABLE ROW LEVEL SECURITY;
 					`,
 						sqlReleaseTableName,
@@ -200,6 +234,12 @@ func (s *SQL) ensureDBSetup() error {
 		},
 	}
 
+	// Check that init migration already applied
+	if s.checkAlreadyApplied(migrations.Migrations) {
+		return nil
+	}
+
+	// Populate the database with the relations we need if they don't exist yet
 	_, err := migrate.Exec(s.db.DB, postgreSQLDialect, migrations, migrate.Up)
 	return err
 }


Test patch:
diff --git a/pkg/storage/driver/sql_test.go b/pkg/storage/driver/sql_test.go
index 4c0c7b668..067b3cf47 100644
--- a/pkg/storage/driver/sql_test.go
+++ b/pkg/storage/driver/sql_test.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	sqlmock "github.com/DATA-DOG/go-sqlmock"
+	migrate "github.com/rubenv/sql-migrate"
 
 	rspb "helm.sh/helm/v3/pkg/release"
 )
@@ -530,3 +531,51 @@ func mockGetReleaseCustomLabels(mock sqlmock.Sqlmock, key string, namespace stri
 	}
 	eq.WillReturnRows(returnRows).RowsWillBeClosed()
 }
+
+func TestSqlChechkAppliedMigrations(t *testing.T) {
+	cases := []struct {
+		migrationsToApply    []*migrate.Migration
+		appliedMigrationsIds []string
+		expectedResult       bool
+		errorExplanation     string
+	}{
+		{
+			migrationsToApply:    []*migrate.Migration{{Id: "init1"}, {Id: "init2"}, {Id: "init3"}},
+			appliedMigrationsIds: []string{"1", "2", "init1", "3", "init2", "4", "5"},
+			expectedResult:       false,
+			errorExplanation:     "Has found one migration id \"init3\" as applied, that was not applied",
+		},
+		{
+			migrationsToApply:    []*migrate.Migration{{Id: "init1"}, {Id: "init2"}, {Id: "init3"}},
+			appliedMigrationsIds: []string{"1", "2", "init1", "3", "init2", "4", "init3", "5"},
+			expectedResult:       true,
+			errorExplanation:     "Has not found one or more migration ids, that was applied",
+		},
+		{
+			migrationsToApply:    []*migrate.Migration{{Id: "init"}},
+			appliedMigrationsIds: []string{"1", "2", "3", "inits", "4", "tinit", "5"},
+			expectedResult:       false,
+			errorExplanation:     "Has found single \"init\", that was not applied",
+		},
+		{
+			migrationsToApply:    []*migrate.Migration{{Id: "init"}},
+			appliedMigrationsIds: []string{"1", "2", "init", "3", "init2", "4", "init3", "5"},
+			expectedResult:       true,
+			errorExplanation:     "Has not found single migration id \"init\", that was applied",
+		},
+	}
+	for i, c := range cases {
+		sqlDriver, mock := newTestFixtureSQL(t)
+		rows := sqlmock.NewRows([]string{"id", "applied_at"})
+		for _, id := range c.appliedMigrationsIds {
+			rows.AddRow(id, time.Time{})
+		}
+		mock.
+			ExpectQuery("").
+			WillReturnRows(rows)
+		mock.ExpectCommit()
+		if sqlDriver.checkAlreadyApplied(c.migrationsToApply) != c.expectedResult {
+			t.Errorf("Test case: %v, Expected: %v, Have: %v, Explanation: %v", i, c.expectedResult, !c.expectedResult, c.errorExplanation)
+		}
+	}
+}