DB migrations for saved scripts (#13765)

changes/issue-9827-migrations-saved-scripts

@@ -0,0 +1 @@
+* Added database migrations to create the new `scripts` table to store saved scripts.

go.mod

@@ -245,6 +245,7 @@ require (
 	github.com/jonboulle/clockwork v0.2.2 // indirect
 	github.com/kevinburke/ssh_config v1.1.0 // indirect
 	github.com/klauspost/compress v1.15.11 // indirect
+	github.com/lib/pq v1.10.9 // indirect
 	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/magiconair/properties v1.8.5 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
@@ -293,6 +294,7 @@ require (
 	github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
 	github.com/yashtewari/glob-intersection v0.1.0 // indirect
 	github.com/yusufpapurcu/wmi v1.2.2 // indirect
+	github.com/ziutek/mymysql v1.5.4 // indirect
 	go.elastic.co/apm/module/apmhttp/v2 v2.3.0 // indirect
 	go.elastic.co/fastjson v1.1.0 // indirect
 	go.opencensus.io v0.24.0 // indirect

go.sum

@@ -849,6 +849,7 @@ github.com/lib/pq v1.9.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lib/pq v1.10.1/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lib/pq v1.10.2/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
+github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
 github.com/macadmins/osquery-extension v0.0.15 h1:uixbimhzKZSguAcLwKAfi0fieB7gIkxm3saPl9mNl9c=
@@ -1209,6 +1210,7 @@ github.com/yusufpapurcu/wmi v1.2.2 h1:KBNDSne4vP5mbSWnJbO+51IMOXJB67QiYCSBrubbPR
 github.com/yusufpapurcu/wmi v1.2.2/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
 github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s=
 github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
+github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=
 github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
 github.com/zwass/kit v0.0.0-20210625184505-ec5b5c5cce9c h1:TWQ2UvXPkhPxI2KmApKBOCaV6yD2N4mlvqFQ/DlPtpQ=
 github.com/zwass/kit v0.0.0-20210625184505-ec5b5c5cce9c/go.mod h1:OYYulo9tUqRadRLwB0+LE914sa1ui2yL7OrcU3Q/1XY=
@@ -1877,4 +1879,4 @@ nhooyr.io/websocket v1.8.7/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
-software.sslmate.com/src/go-pkcs12 v0.0.0-20210415151418-c5206de65a78/go.mod h1:B7Wf0Ya4DHF9Yw+qfZuJijQYkWicqDa+79Ytmmq3Kjg=
+software.sslmate.com/src/go-pkcs12 v0.0.0-20210415151418-c5206de65a78/go.mod h1:B7Wf0Ya4DHF9Yw+qfZuJijQYkWicqDa+79Ytmmq3Kjg=

server/datastore/mysql/migrations/tables/20230906152143_AddScriptsTable.go

@@ -0,0 +1,64 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20230906152143, Down_20230906152143)
+}
+
+func Up_20230906152143(tx *sql.Tx) error {
+	_, err := tx.Exec(`
+CREATE TABLE scripts (
+    id                 INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
+
+    -- team_id NULL is for no team (cannot use 0 with foreign key)
+    team_id            INT(10) UNSIGNED NULL,
+
+    -- this field is 0 for global, and the team_id otherwise, and is
+    -- used for the unique index/constraint (team_id cannot be used
+    -- as it allows NULL).
+    global_or_team_id  INT(10) UNSIGNED NOT NULL DEFAULT 0,
+
+    -- the name of the script file that was uploaded (or applied via fleetctl), note
+    -- that only the filename part of the path is kept and it must be unique for a
+    -- given team/no team.
+    name               VARCHAR(255) NOT NULL,
+
+    -- the contents of the script to execute, length is limited by Fleet.
+    script_contents    TEXT NOT NULL,
+
+    created_at         TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+    updated_at         TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+
+    PRIMARY KEY (id),
+
+    UNIQUE KEY idx_scripts_global_or_team_id_name (global_or_team_id, name),
+    FOREIGN KEY fk_scripts_team_id (team_id) REFERENCES teams (id) ON DELETE CASCADE ON UPDATE CASCADE
+)`)
+	if err != nil {
+		return fmt.Errorf("failed to create scripts table: %w", err)
+	}
+
+	// alter the host_script_results table to add FK to scripts, and on scripts
+	// delete set it to null so that the host script results entry becomes just
+	// like an "anonymous" script execution (a one-off, same as those via fleetctl
+	// run-script) and stop showing up in the list of scripts executions in the
+	// host's page).
+	_, err = tx.Exec(`
+ALTER TABLE host_script_results
+ADD COLUMN script_id INT(10) UNSIGNED NULL DEFAULT NULL,
+ADD CONSTRAINT fk_host_script_results_script_id FOREIGN KEY (script_id) REFERENCES scripts (id) ON DELETE SET NULL
+`)
+	if err != nil {
+		return fmt.Errorf("add script_id to host_script_results: %w", err)
+	}
+
+	return nil
+}
+
+func Down_20230906152143(tx *sql.Tx) error {
+	return nil
+}

server/datastore/mysql/migrations/tables/20230906152143_AddScriptsTable_test.go

@@ -0,0 +1,148 @@
+package tables
+
+import (
+	"database/sql"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20230906152143(t *testing.T) {
+	db := applyUpToPrev(t)
+
+	const (
+		insertOneOffResultStmt = `INSERT INTO host_script_results (
+			host_id, execution_id, script_contents, output
+		) VALUES (?, ?, ?, '')`
+
+		insertScriptResultStmt = `INSERT INTO host_script_results (
+			host_id, execution_id, script_contents, script_id, output
+		) VALUES (?, ?, ?, ?, '')`
+
+		insertScriptStmt = `INSERT INTO scripts (
+			team_id, global_or_team_id, name, script_contents
+		) VALUES (?, ?, ?, ?)`
+
+		insertTeamStmt = `INSERT INTO teams (name) VALUES (?)`
+
+		deleteScriptStmt = `DELETE FROM scripts WHERE id = ?`
+
+		deleteTeamStmt = `DELETE FROM teams WHERE id = ?`
+
+		loadResultStmt = `SELECT
+			id, host_id, execution_id, script_contents, script_id
+		FROM host_script_results WHERE id = ?`
+
+		loadScriptStmt = `SELECT id FROM scripts WHERE id = ?`
+	)
+
+	type script struct {
+		id, globalOrTeamID   int64
+		name, scriptContents string
+		teamID               *int64
+	}
+	type scriptResult struct {
+		id, hostID                  int64
+		executionID, scriptContents string
+		scriptID                    *int64
+	}
+
+	// create an existing (one-off) host script results (using maps to avoid
+	// referencing structs that may change in the future)
+	preExistingResult := scriptResult{
+		hostID:         123,
+		executionID:    uuid.New().String(),
+		scriptContents: "a",
+	}
+	res, err := db.Exec(insertOneOffResultStmt, preExistingResult.hostID, preExistingResult.executionID, preExistingResult.scriptContents)
+	require.NoError(t, err)
+	preExistingResult.id, _ = res.LastInsertId()
+
+	// Apply current migration.
+	applyNext(t, db)
+
+	// create a global script
+	globalScript := script{
+		globalOrTeamID: 0,
+		teamID:         nil,
+		name:           "global-script",
+		scriptContents: "b",
+	}
+	res, err = db.Exec(insertScriptStmt, globalScript.teamID, globalScript.globalOrTeamID, globalScript.name, globalScript.scriptContents)
+	require.NoError(t, err)
+	globalScript.id, _ = res.LastInsertId()
+
+	// create a host script result for that global script
+	globalScriptResult := scriptResult{
+		hostID:         123,
+		executionID:    uuid.New().String(),
+		scriptContents: globalScript.scriptContents,
+		scriptID:       &globalScript.id,
+	}
+	res, err = db.Exec(insertScriptResultStmt, globalScriptResult.hostID, globalScriptResult.executionID, globalScriptResult.scriptContents, globalScriptResult.scriptID)
+	require.NoError(t, err)
+	globalScriptResult.id, _ = res.LastInsertId()
+
+	// delete the global script
+	_, err = db.Exec(deleteScriptStmt, globalScript.id)
+	require.NoError(t, err)
+
+	// the global host script result is still present but now unlinked to the script
+	var result scriptResult
+	err = db.QueryRow(loadResultStmt, globalScriptResult.id).Scan(&result.id, &result.hostID, &result.executionID, &result.scriptContents, &result.scriptID)
+	require.NoError(t, err)
+	require.Nil(t, result.scriptID)
+	// clear the script id on globalScriptResult to allow comparing the rest of the fields
+	globalScriptResult.scriptID = nil
+	require.Equal(t, globalScriptResult, result)
+
+	// create a team-specific script
+	res, err = db.Exec(insertTeamStmt, "team1")
+	require.NoError(t, err)
+	teamID, _ := res.LastInsertId()
+
+	teamScript := script{
+		globalOrTeamID: teamID,
+		teamID:         &teamID,
+		name:           "team-script",
+		scriptContents: "c",
+	}
+	res, err = db.Exec(insertScriptStmt, teamScript.teamID, teamScript.globalOrTeamID, teamScript.name, teamScript.scriptContents)
+	require.NoError(t, err)
+	teamScript.id, _ = res.LastInsertId()
+
+	// create a host script result for that team script
+	teamScriptResult := scriptResult{
+		hostID:         123,
+		executionID:    uuid.New().String(),
+		scriptContents: teamScript.scriptContents,
+		scriptID:       &teamScript.id,
+	}
+	res, err = db.Exec(insertScriptResultStmt, teamScriptResult.hostID, teamScriptResult.executionID, teamScriptResult.scriptContents, teamScriptResult.scriptID)
+	require.NoError(t, err)
+	teamScriptResult.id, _ = res.LastInsertId()
+
+	// delete the team
+	_, err = db.Exec(deleteTeamStmt, teamID)
+	require.NoError(t, err)
+
+	// the script is deleted, but the host script result still exists (unlinked to the script)
+	var notFoundID int64
+	err = db.QueryRow(loadScriptStmt, teamScript.id).Scan(&notFoundID)
+	require.Error(t, err)
+	require.ErrorIs(t, err, sql.ErrNoRows)
+
+	err = db.QueryRow(loadResultStmt, teamScriptResult.id).Scan(&result.id, &result.hostID, &result.executionID, &result.scriptContents, &result.scriptID)
+	require.NoError(t, err)
+	require.Nil(t, result.scriptID)
+	// clear the script id on teamScriptResult to allow comparing the rest of the fields
+	teamScriptResult.scriptID = nil
+	require.Equal(t, teamScriptResult, result)
+
+	// the pre-existing host script result is still there, untouched
+	err = db.QueryRow(loadResultStmt, preExistingResult.id).Scan(&result.id, &result.hostID, &result.executionID, &result.scriptContents, &result.scriptID)
+	require.NoError(t, err)
+	require.Nil(t, result.scriptID)
+	require.Equal(t, preExistingResult, result)
+}