pr comments

coder · f0ssel · Sep 16, 2024 · Sep 9, 2024 · Sep 9, 2024 · Sep 10, 2024
commit c0ae05624723b5f673a9ea6f61bb2e26dd18633f
diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go
@@ -544,11 +544,6 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
 func RecentProvisionerDaemons(now time.Time, staleInterval time.Duration, daemons []database.ProvisionerDaemon) []codersdk.ProvisionerDaemon {
 	results := []codersdk.ProvisionerDaemon{}
 
-	// Ensure stable order for display and for tests
-	sort.Slice(daemons, func(i, j int) bool {
-		return daemons[i].Name < daemons[j].Name
-	})
-
 	for _, daemon := range daemons {
 		// Daemon never connected, skip.
 		if !daemon.LastSeenAt.Valid {
@@ -562,6 +557,11 @@ func RecentProvisionerDaemons(now time.Time, staleInterval time.Duration, daemon
 		results = append(results, ProvisionerDaemon(daemon))
 	}
 
+	// Ensure stable order for display and for tests
+	sort.Slice(results, func(i, j int) bool {
+		return results[i].Name < results[j].Name
+	})
+
 	return results
 }
 

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -115,7 +115,7 @@ func New() database.Store {
 		OrganizationID: defaultOrg.ID,
 		CreatedAt:      dbtime.Now(),
 		HashedSecret:   []byte{},
-		Name:           "built-in",
+		Name:           codersdk.ProvisionerKeyNameBuiltIn,
 		Tags:           map[string]string{},
 	})
 	if err != nil {
@@ -126,7 +126,7 @@ func New() database.Store {
 		OrganizationID: defaultOrg.ID,
 		CreatedAt:      dbtime.Now(),
 		HashedSecret:   []byte{},
-		Name:           "user-auth",
+		Name:           codersdk.ProvisionerKeyNameUserAuth,
 		Tags:           map[string]string{},
 	})
 	if err != nil {
@@ -137,7 +137,7 @@ func New() database.Store {
 		OrganizationID: defaultOrg.ID,
 		CreatedAt:      dbtime.Now(),
 		HashedSecret:   []byte{},
-		Name:           "psk",
+		Name:           codersdk.ProvisionerKeyNamePSK,
 		Tags:           map[string]string{},
 	})
 	if err != nil {

diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/coder/coder/v2/coderd/database/dbrollup"
 	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
+	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/provisionerd/proto"
 	"github.com/coder/coder/v2/provisionersdk"
 	"github.com/coder/coder/v2/testutil"
@@ -412,6 +413,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) {
 		Version:        "1.0.0",
 		APIVersion:     proto.CurrentVersion.String(),
 		OrganizationID: defaultOrg.ID,
+		KeyID:          uuid.MustParse(codersdk.ProvisionerKeyIDBuiltIn),
 	})
 	require.NoError(t, err)
 	_, err = db.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
@@ -424,6 +426,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) {
 		Version:        "1.0.0",
 		APIVersion:     proto.CurrentVersion.String(),
 		OrganizationID: defaultOrg.ID,
+		KeyID:          uuid.MustParse(codersdk.ProvisionerKeyIDBuiltIn),
 	})
 	require.NoError(t, err)
 	_, err = db.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
@@ -438,6 +441,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) {
 		Version:        "1.0.0",
 		APIVersion:     proto.CurrentVersion.String(),
 		OrganizationID: defaultOrg.ID,
+		KeyID:          uuid.MustParse(codersdk.ProvisionerKeyIDBuiltIn),
 	})
 	require.NoError(t, err)
 	_, err = db.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
@@ -453,6 +457,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) {
 		Version:        "1.0.0",
 		APIVersion:     proto.CurrentVersion.String(),
 		OrganizationID: defaultOrg.ID,
+		KeyID:          uuid.MustParse(codersdk.ProvisionerKeyIDBuiltIn),
 	})
 	require.NoError(t, err)
 

diff --git a/coderd/database/queries/provisionerkeys.sql b/coderd/database/queries/provisionerkeys.sql
@@ -45,10 +45,13 @@ FROM
 WHERE
     organization_id = $1
 AND
+    -- exclude reserved built-in key
     id != '11111111-1111-1111-1111-111111111111'::uuid
 AND 
+    -- exclude reserved user-auth key
     id != '22222222-2222-2222-2222-222222222222'::uuid
 AND 
+    -- exclude reserved psk key
     id != '33333333-3333-3333-3333-333333333333'::uuid;
 
 -- name: DeleteProvisionerKey :exec

diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go
@@ -289,6 +289,20 @@ const (
 	ProvisionerKeyIDPSK      = "33333333-3333-3333-3333-333333333333"
 )
 
+const (
+	ProvisionerKeyNameBuiltIn  = "built-in"
+	ProvisionerKeyNameUserAuth = "user-auth"
+	ProvisionerKeyNamePSK      = "psk"
+)
+
+func ReservedProvisionerKeyNames() []string {
+	return []string{
+		ProvisionerKeyNameBuiltIn,
+		ProvisionerKeyNameUserAuth,
+		ProvisionerKeyNamePSK,
+	}
+}
+
 type CreateProvisionerKeyRequest struct {
 	Name string            `json:"name"`
 	Tags map[string]string `json:"tags"`

diff --git a/enterprise/coderd/provisionerkeys.go b/enterprise/coderd/provisionerkeys.go
@@ -55,8 +55,7 @@ func (api *API) postProvisionerKey(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	reserved := []string{"built-in", "psk", "user-auth"}
-	if slices.Contains(reserved, req.Name) {
+	if slices.Contains(codersdk.ReservedProvisionerKeyNames(), req.Name) {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: fmt.Sprintf("Name cannot be reserved name '%s'", req.Name),
 			Validations: []codersdk.ValidationError{

diff --git a/enterprise/coderd/provisionerkeys_test.go b/enterprise/coderd/provisionerkeys_test.go
@@ -66,15 +66,15 @@ func TestProvisionerKeys(t *testing.T) {
 
 	// org admin cannot create reserved provisioner keys
 	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
-		Name: "built-in",
+		Name: codersdk.ProvisionerKeyNameBuiltIn,
 	})
 	require.ErrorContains(t, err, "reserved")
 	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
-		Name: "user-auth",
+		Name: codersdk.ProvisionerKeyNameUserAuth,
 	})
 	require.ErrorContains(t, err, "reserved")
 	_, err = orgAdmin.CreateProvisionerKey(ctx, owner.OrganizationID, codersdk.CreateProvisionerKeyRequest{
-		Name: "psk",
+		Name: codersdk.ProvisionerKeyNamePSK,
 	})
 	require.ErrorContains(t, err, "reserved")
 
@@ -127,10 +127,10 @@ func TestProvisionerKeys(t *testing.T) {
 	require.ErrorContains(t, err, "Resource not found")
 
 	// org admin cannot delete reserved provisioner keys
-	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, "built-in")
+	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, codersdk.ProvisionerKeyNameBuiltIn)
 	require.ErrorContains(t, err, "reserved")
-	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, "user-auth")
+	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, codersdk.ProvisionerKeyNameUserAuth)
 	require.ErrorContains(t, err, "reserved")
-	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, "psk")
+	err = orgAdmin.DeleteProvisionerKey(ctx, owner.OrganizationID, codersdk.ProvisionerKeyNamePSK)
 	require.ErrorContains(t, err, "reserved")
 }