feat: Remove organization and user scoped parameters

Signed-off-by: Spike Curtis <spike@coder.com>
coder · spikecurtis · Jun 3, 2022 · Jun 2, 2022 · Jun 2, 2022 · Jun 3, 2022
commit 7edbe984fe41b6ddbe459fde515b8da5e92c2c3e
diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
@@ -61,14 +61,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 	organization, err := client.Organization(ctx, admin.OrganizationID)
 	require.NoError(t, err, "fetch org")
 
-	organizationParam, err := client.CreateParameter(ctx, codersdk.ParameterOrganization, organization.ID, codersdk.CreateParameterRequest{
-		Name:              "test-param",
-		SourceValue:       "hello world",
-		SourceScheme:      codersdk.ParameterSourceSchemeData,
-		DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
-	})
-	require.NoError(t, err, "create org param")
-
 	// Setup some data in the database.
 	version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, &echo.Responses{
 		Parse: echo.ParseComplete,
@@ -101,6 +93,14 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 	})
 	require.NoError(t, err, "template version dry-run")
 
+	templateParam, err := client.CreateParameter(ctx, codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
+		Name:              "test-param",
+		SourceValue:       "hello world",
+		SourceScheme:      codersdk.ParameterSourceSchemeData,
+		DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
+	})
+	require.NoError(t, err, "create template param")
+
 	// Always fail auth from this point forward
 	authorizer.AlwaysReturn = rbac.ForbiddenWithInternal(xerrors.New("fake implementation"), nil, nil)
 
@@ -294,15 +294,15 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 
 		"POST:/api/v2/parameters/{scope}/{id}": {
 			AssertAction: rbac.ActionUpdate,
-			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+			AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
 		},
 		"GET:/api/v2/parameters/{scope}/{id}": {
 			AssertAction: rbac.ActionRead,
-			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+			AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
 		},
 		"DELETE:/api/v2/parameters/{scope}/{id}/{name}": {
 			AssertAction: rbac.ActionUpdate,
-			AssertObject: rbac.ResourceOrganization.WithID(organization.ID.String()),
+			AssertObject: rbac.ResourceTemplate.WithID(template.ID.String()),
 		},
 		"GET:/api/v2/organizations/{organization}/templates/{templatename}": {
 			AssertAction: rbac.ActionRead,
@@ -377,9 +377,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 			route = strings.ReplaceAll(route, "{templateversion}", version.ID.String())
 			route = strings.ReplaceAll(route, "{templateversiondryrun}", templateVersionDryRun.ID.String())
 			route = strings.ReplaceAll(route, "{templatename}", template.Name)
-			// Only checking org scoped params here
-			route = strings.ReplaceAll(route, "{scope}", string(organizationParam.Scope))
-			route = strings.ReplaceAll(route, "{id}", organizationParam.ScopeID.String())
+			// Only checking template scoped params here
+			route = strings.ReplaceAll(route, "{scope}", string(templateParam.Scope))
+			route = strings.ReplaceAll(route, "{id}", templateParam.ScopeID.String())
 
 			resp, err := client.Request(context.Background(), method, route, nil)
 			require.NoError(t, err, "do req")

diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000019_drop_org_user_parameter_scope.down.sql b/coderd/database/migrations/000019_drop_org_user_parameter_scope.down.sql
@@ -0,0 +1,2 @@
+ALTER TYPE parameter_scope ADD VALUE 'organization';
+ALTER TYPE parameter_scope ADD VALUE 'user';
diff --git a/coderd/database/migrations/000019_drop_org_user_parameter_scope.up.sql b/coderd/database/migrations/000019_drop_org_user_parameter_scope.up.sql
@@ -0,0 +1,11 @@
+-- We no longer support org or user scoped values, so delete them
+DELETE FROM parameter_values WHERE scope IN ('organization', 'user');
+
+CREATE TYPE new_parameter_scope AS ENUM (
+     'template',
+     'import_job',
+     'workspace'
+);
+ALTER TABLE parameter_values ALTER COLUMN scope TYPE new_parameter_scope USING (scope::text::new_parameter_scope);
+DROP TYPE parameter_scope;
+ALTER TYPE new_parameter_scope RENAME TO parameter_scope;
diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/coderd/parameter/compute.go b/coderd/parameter/compute.go
@@ -60,15 +60,6 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options
 		compute.parameterSchemasByName[parameterSchema.Name] = parameterSchema
 	}
 
-	// Organization parameters come first!
-	err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{
-		Scope:   database.ParameterScopeOrganization,
-		ScopeID: scope.OrganizationID,
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	// Job parameters come second!
 	err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{
 		Scope:   database.ParameterScopeImportJob,
@@ -123,15 +114,6 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options
 		}
 	}
 
-	// User parameters come fourth!
-	err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{
-		Scope:   database.ParameterScopeUser,
-		ScopeID: scope.UserID,
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	if scope.WorkspaceID.Valid {
 		// Workspace parameters come last!
 		err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{
@@ -194,9 +176,8 @@ func (c *compute) injectSingle(scopedParameter database.ParameterValue, defaultV
 	_, hasParameterValue := c.computedParameterByName[scopedParameter.Name]
 	if hasParameterValue {
 		if !parameterSchema.AllowOverrideSource &&
-			// Users and workspaces cannot override anything on a template!
-			(scopedParameter.Scope == database.ParameterScopeUser ||
-				scopedParameter.Scope == database.ParameterScopeWorkspace) {
+			// Workspaces cannot override anything on a template!
+			scopedParameter.Scope == database.ParameterScopeWorkspace {
 			return nil
 		}
 	}

diff --git a/coderd/parameter/compute_test.go b/coderd/parameter/compute_test.go
@@ -92,42 +92,6 @@ func TestCompute(t *testing.T) {
 		require.Equal(t, computedValue.SourceValue, parameterSchema.DefaultSourceValue)
 	})
 
-	t.Run("OverrideOrganizationWithImportJob", func(t *testing.T) {
-		t.Parallel()
-		db := databasefake.New()
-		scope := generateScope()
-		parameterSchema := generateParameter(t, db, parameterOptions{
-			TemplateImportJobID: scope.TemplateImportJobID,
-		})
-		_, err := db.InsertParameterValue(context.Background(), database.InsertParameterValueParams{
-			ID:                uuid.New(),
-			Name:              parameterSchema.Name,
-			Scope:             database.ParameterScopeOrganization,
-			ScopeID:           scope.OrganizationID,
-			SourceScheme:      database.ParameterSourceSchemeData,
-			SourceValue:       "firstnop",
-			DestinationScheme: database.ParameterDestinationSchemeEnvironmentVariable,
-		})
-		require.NoError(t, err)
-
-		value, err := db.InsertParameterValue(context.Background(), database.InsertParameterValueParams{
-			ID:                uuid.New(),
-			Name:              parameterSchema.Name,
-			Scope:             database.ParameterScopeImportJob,
-			ScopeID:           scope.TemplateImportJobID,
-			SourceScheme:      database.ParameterSourceSchemeData,
-			SourceValue:       "secondnop",
-			DestinationScheme: database.ParameterDestinationSchemeEnvironmentVariable,
-		})
-		require.NoError(t, err)
-
-		computed, err := parameter.Compute(context.Background(), db, scope, nil)
-		require.NoError(t, err)
-		require.Len(t, computed, 1)
-		require.Equal(t, false, computed[0].DefaultSourceValue)
-		require.Equal(t, value.SourceValue, computed[0].SourceValue)
-	})
-
 	t.Run("TemplateOverridesTemplateDefault", func(t *testing.T) {
 		t.Parallel()
 		db := databasefake.New()

diff --git a/coderd/parameters.go b/coderd/parameters.go
@@ -210,16 +210,6 @@ func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, s
 		resource, err = api.Database.GetWorkspaceByID(ctx, scopeID)
 	case database.ParameterScopeTemplate:
 		resource, err = api.Database.GetTemplateByID(ctx, scopeID)
-	case database.ParameterScopeOrganization:
-		resource, err = api.Database.GetOrganizationByID(ctx, scopeID)
-	case database.ParameterScopeUser:
-		user, userErr := api.Database.GetUserByID(ctx, scopeID)
-		err = userErr
-		if err != nil {
-			// Use the userdata resource instead of the user. This way users
-			// can add user scoped params.
-			resource = rbac.ResourceUserData.WithID(user.ID.String()).WithOwner(user.ID.String())
-		}
 	case database.ParameterScopeImportJob:
 		// This scope does not make sense from this api.
 		// ImportJob params are created with the job, and the job id cannot
@@ -246,12 +236,8 @@ func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, s
 func readScopeAndID(rw http.ResponseWriter, r *http.Request) (database.ParameterScope, uuid.UUID, bool) {
 	var scope database.ParameterScope
 	switch chi.URLParam(r, "scope") {
-	case string(codersdk.ParameterOrganization):
-		scope = database.ParameterScopeOrganization
 	case string(codersdk.ParameterTemplate):
 		scope = database.ParameterScopeTemplate
-	case string(codersdk.ParameterUser):
-		scope = database.ParameterScopeUser
 	case string(codersdk.ParameterWorkspace):
 		scope = database.ParameterScopeWorkspace
 	default:

diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go
@@ -2,6 +2,8 @@ package coderd_test
 
 import (
 	"context"
+	"github.com/coder/coder/provisioner/echo"
+	"github.com/coder/coder/provisionersdk/proto"
 	"net/http"
 	"testing"
 
@@ -32,7 +34,8 @@ func TestPostParameter(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.CreateParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, codersdk.CreateParameterRequest{
+		template := createTemplate(t, client, user)
+		_, err := client.CreateParameter(context.Background(), codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
 			Name:              "example",
 			SourceValue:       "tomato",
 			SourceScheme:      codersdk.ParameterSourceSchemeData,
@@ -45,15 +48,16 @@ func TestPostParameter(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.CreateParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, codersdk.CreateParameterRequest{
+		template := createTemplate(t, client, user)
+		_, err := client.CreateParameter(context.Background(), codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
 			Name:              "example",
 			SourceValue:       "tomato",
 			SourceScheme:      codersdk.ParameterSourceSchemeData,
 			DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
 		})
 		require.NoError(t, err)
 
-		_, err = client.CreateParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, codersdk.CreateParameterRequest{
+		_, err = client.CreateParameter(context.Background(), codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
 			Name:              "example",
 			SourceValue:       "tomato",
 			SourceScheme:      codersdk.ParameterSourceSchemeData,
@@ -71,21 +75,23 @@ func TestParameters(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.Parameters(context.Background(), codersdk.ParameterOrganization, user.OrganizationID)
+		template := createTemplate(t, client, user)
+		_, err := client.Parameters(context.Background(), codersdk.ParameterTemplate, template.ID)
 		require.NoError(t, err)
 	})
 	t.Run("List", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.CreateParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, codersdk.CreateParameterRequest{
+		template := createTemplate(t, client, user)
+		_, err := client.CreateParameter(context.Background(), codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
 			Name:              "example",
 			SourceValue:       "tomato",
 			SourceScheme:      codersdk.ParameterSourceSchemeData,
 			DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
 		})
 		require.NoError(t, err)
-		params, err := client.Parameters(context.Background(), codersdk.ParameterOrganization, user.OrganizationID)
+		params, err := client.Parameters(context.Background(), codersdk.ParameterTemplate, template.ID)
 		require.NoError(t, err)
 		require.Len(t, params, 1)
 	})
@@ -97,7 +103,8 @@ func TestDeleteParameter(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		err := client.DeleteParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, "something")
+		template := createTemplate(t, client, user)
+		err := client.DeleteParameter(context.Background(), codersdk.ParameterTemplate, template.ID, "something")
 		var apiErr *codersdk.Error
 		require.ErrorAs(t, err, &apiErr)
 		require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
@@ -106,14 +113,39 @@ func TestDeleteParameter(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
-		param, err := client.CreateParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, codersdk.CreateParameterRequest{
+		template := createTemplate(t, client, user)
+		param, err := client.CreateParameter(context.Background(), codersdk.ParameterTemplate, template.ID, codersdk.CreateParameterRequest{
 			Name:              "example",
 			SourceValue:       "tomato",
 			SourceScheme:      codersdk.ParameterSourceSchemeData,
 			DestinationScheme: codersdk.ParameterDestinationSchemeProvisionerVariable,
 		})
 		require.NoError(t, err)
-		err = client.DeleteParameter(context.Background(), codersdk.ParameterOrganization, user.OrganizationID, param.Name)
+		err = client.DeleteParameter(context.Background(), codersdk.ParameterTemplate, template.ID, param.Name)
 		require.NoError(t, err)
 	})
 }
+
+func createTemplate(t *testing.T, client *codersdk.Client, user codersdk.CreateFirstUserResponse) codersdk.Template {
+	instanceID := "instanceidentifier"
+	version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
+		Parse: echo.ParseComplete,
+		Provision: []*proto.Provision_Response{{
+			Type: &proto.Provision_Response_Complete{
+				Complete: &proto.Provision_Complete{
+					Resources: []*proto.Resource{{
+						Name: "somename",
+						Type: "someinstance",
+						Agents: []*proto.Agent{{
+							Auth: &proto.Agent_InstanceId{
+								InstanceId: instanceID,
+							},
+						}},
+					}},
+				},
+			},
+		}},
+	})
+	template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+	return template
+}
diff --git a/codersdk/parameters.go b/codersdk/parameters.go
@@ -14,10 +14,8 @@ import (
 type ParameterScope string
 
 const (
-	ParameterOrganization ParameterScope = "organization"
-	ParameterTemplate     ParameterScope = "template"
-	ParameterUser         ParameterScope = "user"
-	ParameterWorkspace    ParameterScope = "workspace"
+	ParameterTemplate  ParameterScope = "template"
+	ParameterWorkspace ParameterScope = "workspace"
 )
 
 type ParameterSourceScheme string
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		ALTER TYPE parameter_scope ADD VALUE 'organization';
		ALTER TYPE parameter_scope ADD VALUE 'user';