Merge branch 'main' into fixmetadataorder

kylecarbs · kylecarbs · commit 825f698caa11 · 2022-12-14T18:42:53.000Z
diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml
@@ -338,7 +338,7 @@ jobs:
           else
             echo ::set-output name=cover::false
           fi
-          gotestsum --junitfile="gotests.xml" --jsonfile="gotestsum.json" --packages="./..." --debug -- -parallel=8 -timeout=3m -short -failfast $COVERAGE_FLAGS
+          gotestsum --junitfile="gotests.xml" --jsonfile="gotestsum.json" --packages="./..." --debug -- -parallel=8 -timeout=5m -short -failfast $COVERAGE_FLAGS
           ret=$?
           if ((ret)); then
             # Eternalize test timeout logs because "re-run failed" erases
diff --git a/agent/agent.go b/agent/agent.go
@@ -226,6 +226,25 @@ func (a *agent) run(ctx context.Context) error {
 			_ = network.Close()
 			return xerrors.New("agent is closed")
 		}
+
+		// Report statistics from the created network.
+		cl, err := a.client.AgentReportStats(ctx, a.logger, func() *codersdk.AgentStats {
+			stats := network.ExtractTrafficStats()
+			return convertAgentStats(stats)
+		})
+		if err != nil {
+			a.logger.Error(ctx, "report stats", slog.Error(err))
+		} else {
+			if err = a.trackConnGoroutine(func() {
+				// This is OK because the agent never re-creates the tailnet
+				// and the only shutdown indicator is agent.Close().
+				<-a.closed
+				_ = cl.Close()
+			}); err != nil {
+				a.logger.Debug(ctx, "report stats goroutine", slog.Error(err))
+				_ = cl.Close()
+			}
+		}
 	} else {
 		// Update the DERP map!
 		network.SetDERPMap(metadata.DERPMap)
@@ -561,28 +580,6 @@ func (a *agent) init(ctx context.Context) {
 	}
 
 	go a.runLoop(ctx)
-	cl, err := a.client.AgentReportStats(ctx, a.logger, func() *codersdk.AgentStats {
-		stats := map[netlogtype.Connection]netlogtype.Counts{}
-		a.closeMutex.Lock()
-		if a.network != nil {
-			stats = a.network.ExtractTrafficStats()
-		}
-		a.closeMutex.Unlock()
-		return convertAgentStats(stats)
-	})
-	if err != nil {
-		a.logger.Error(ctx, "report stats", slog.Error(err))
-		return
-	}
-
-	if err = a.trackConnGoroutine(func() {
-		<-a.closed
-		_ = cl.Close()
-	}); err != nil {
-		a.logger.Error(ctx, "report stats goroutine", slog.Error(err))
-		_ = cl.Close()
-		return
-	}
 }
 
 func convertAgentStats(counts map[netlogtype.Connection]netlogtype.Counts) *codersdk.AgentStats {
diff --git a/coderd/database/migrations/000086_no_org_admins.down.sql b/coderd/database/migrations/000086_no_org_admins.down.sql
diff --git a/coderd/database/migrations/000086_no_org_admins.up.sql b/coderd/database/migrations/000086_no_org_admins.up.sql
@@ -0,0 +1,6 @@
+UPDATE 
+  organization_members
+SET 
+  roles = ARRAY [] :: text[]
+WHERE 
+  'organization-admin:'||organization_id = ANY(roles);
diff --git a/coderd/database/migrations/000086_resource_metadata_order.down.sql b/coderd/database/migrations/000086_resource_metadata_order.down.sql
diff --git a/coderd/database/migrations/000087_resource_metadata_order.down.sql b/coderd/database/migrations/000087_resource_metadata_order.down.sql
@@ -0,0 +1,4 @@
+ALTER TABLE workspace_resource_metadata DROP COLUMN id;
+ALTER TABLE workspace_resource_metadata DROP CONSTRAINT workspace_resource_metadata_name;
+ALTER TABLE workspace_resource_metadata ADD CONSTRAINT workspace_resource_metadata_pkey PRIMARY KEY (workspace_resource_id, key);
+
diff --git a/coderd/database/migrations/000087_resource_metadata_order.up.sql b/coderd/database/migrations/000087_resource_metadata_order.up.sql
diff --git a/coderd/httpmw/workspaceagent.go b/coderd/httpmw/workspaceagent.go
@@ -30,25 +30,27 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 			ctx := r.Context()
-			cookieValue := apiTokenFromRequest(r)
-			if cookieValue == "" {
+			tokenValue := apiTokenFromRequest(r)
+			if tokenValue == "" {
 				httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
 					Message: fmt.Sprintf("Cookie %q must be provided.", codersdk.SessionTokenKey),
 				})
 				return
 			}
-			token, err := uuid.Parse(cookieValue)
+			token, err := uuid.Parse(tokenValue)
 			if err != nil {
 				httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
-					Message: "Agent token is invalid.",
+					Message: "Workspace agent token invalid.",
+					Detail:  fmt.Sprintf("An agent token must be a valid UUIDv4. (len %d)", len(tokenValue)),
 				})
 				return
 			}
 			agent, err := db.GetWorkspaceAgentByAuthToken(ctx, token)
 			if err != nil {
 				if errors.Is(err, sql.ErrNoRows) {
 					httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
-						Message: "Agent token is invalid.",
+						Message: "Workspace agent not authorized.",
+						Detail:  "The agent cannot authenticate until the workspace provision job has been completed. If the job is no longer running, this agent is invalid.",
 					})
 					return
 				}
diff --git a/coderd/organizations.go b/coderd/organizations.go
@@ -76,7 +76,11 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
 			CreatedAt:      database.Now(),
 			UpdatedAt:      database.Now(),
 			Roles: []string{
-				rbac.RoleOrgAdmin(organization.ID),
+				// TODO: When organizations are allowed to be created, we should
+				// come back to determining the default role of the person who
+				// creates the org. Until that happens, all users in an organization
+				// should be just regular members.
+				rbac.RoleOrgMember(organization.ID),
 			},
 		})
 		if err != nil {
diff --git a/coderd/users.go b/coderd/users.go
@@ -1071,7 +1071,11 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 				return xerrors.Errorf("create organization: %w", err)
 			}
 			req.OrganizationID = organization.ID
-			orgRoles = append(orgRoles, rbac.RoleOrgAdmin(req.OrganizationID))
+			// TODO: When organizations are allowed to be created, we should
+			// come back to determining the default role of the person who
+			// creates the org. Until that happens, all users in an organization
+			// should be just regular members.
+			orgRoles = append(orgRoles, rbac.RoleOrgMember(req.OrganizationID))
 
 			_, err = tx.InsertAllUsersGroup(ctx, organization.ID)
 			if err != nil {
diff --git a/coderd/users_test.go b/coderd/users_test.go
@@ -817,15 +817,6 @@ func TestGrantSiteRoles(t *testing.T) {
 			Error:        true,
 			StatusCode:   http.StatusForbidden,
 		},
-		{
-			Name:         "MemberAssignMember",
-			Client:       member,
-			OrgID:        first.OrganizationID,
-			AssignToUser: first.UserID.String(),
-			Roles:        []string{},
-			Error:        true,
-			StatusCode:   http.StatusForbidden,
-		},
 		{
 			Name:         "AdminUpdateOrgSelf",
 			Client:       admin,
@@ -921,7 +912,7 @@ func TestInitialRoles(t *testing.T) {
 	}, "should be a member and admin")
 
 	require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], []string{
-		rbac.RoleOrgAdmin(first.OrganizationID),
+		rbac.RoleOrgMember(first.OrganizationID),
 	}, "should be a member and admin")
 }
 
diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go
@@ -218,8 +218,15 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
 				if agent.Id != agentID {
 					continue
 				}
-				agent.Auth = &proto.Agent_InstanceId{
-					InstanceId: instanceID,
+				// Only apply the instance ID if the agent authentication
+				// type is set to do so. A user ran into a bug where they
+				// had the instance ID block, but auth was set to "token". See:
+				// https://github.com/coder/coder/issues/4551#issuecomment-1336293468
+				switch t := agent.Auth.(type) {
+				case *proto.Agent_Token:
+					continue
+				case *proto.Agent_InstanceId:
+					t.InstanceId = instanceID
 				}
 				break
 			}
