Merge branch 'main' into pnpm-fixup

coder · aslilac · Aug 2, 2024 · Aug 2, 2024 · Aug 8, 2024 · Aug 8, 2024
commit b0cec4279ac950aa9b0ec319c4f2080df4b58864
diff --git a/.github/actions/setup-node/action.yaml b/.github/actions/setup-node/action.yaml
@@ -13,11 +13,11 @@ runs:
     - name: Install pnpm
       uses: pnpm/action-setup@v3
       with:
-        version: 9
+        version: 9.6
     - name: Setup Node
-      uses: actions/setup-node@v4.0.1
+      uses: actions/setup-node@v4.0.3
       with:
-        node-version: 18.19.0
+        node-version: 20.16.0
         # See https://github.com/actions/setup-node#caching-global-packages-data
         cache: "pnpm"
         cache-dependency-path: ${{ inputs.directory }}/pnpm-lock.yaml

diff --git a/.github/dependabot.yaml b/.github/dependabot.yaml
@@ -86,37 +86,30 @@ updates:
           - "@mui*"
       react:
         patterns:
-          - "react*"
-          - "@types/react*"
+          - "react"
+          - "react-dom"
+          - "@types/react"
+          - "@types/react-dom"
       emotion:
         patterns:
           - "@emotion*"
+        exclude-patterns:
+          - "jest-runner-eslint"
       eslint:
         patterns:
           - "eslint*"
           - "@typescript-eslint*"
       jest:
         patterns:
-          - "jest*"
+          - "jest"
           - "@types/jest"
       vite:
         patterns:
           - "vite*"
           - "@vitejs/plugin-react"
     ignore:
-      # Ignore patch updates for all dependencies
+      # Ignore major version updates to avoid breaking changes
       - dependency-name: "*"
-        update-types:
-          - version-update:semver-patch
-      # Ignore major updates to Node.js types, because they need to
-      # correspond to the Node.js engine version
-      - dependency-name: "@types/node"
         update-types:
           - version-update:semver-major
-      # Ignore @storybook updates, run `pnpm dlx storybook@latest upgrade` to upgrade manually
-      - dependency-name: "*storybook*" # matches @storybook/* and storybook*
-        update-types:
-          - version-update:semver-major
-          - version-update:semver-minor
-          - version-update:semver-patch
     open-pull-requests-limit: 15
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -184,7 +184,7 @@ jobs:
 
       # Check for any typos
       - name: Check for typos
-        uses: crate-ci/typos@v1.23.5
+        uses: crate-ci/typos@v1.23.6
         with:
           config: .github/workflows/typos.toml
 
@@ -709,7 +709,6 @@ jobs:
       - test-e2e
       - offlinedocs
       - sqlc-vet
-      - dependency-license-review
     # Allow this job to run even if the needed jobs fail, are skipped or
     # cancelled.
     if: always()
@@ -726,7 +725,6 @@ jobs:
           echo "- test-js: ${{ needs.test-js.result }}"
           echo "- test-e2e: ${{ needs.test-e2e.result }}"
           echo "- offlinedocs: ${{ needs.offlinedocs.result }}"
-          echo "- dependency-license-review: ${{ needs.dependency-license-review.result }}"
           echo
 
           # We allow skipped jobs to pass, but not failed or cancelled jobs.
@@ -968,43 +966,3 @@ jobs:
       - name: Setup and run sqlc vet
         run: |
           make sqlc-vet
-
-  # dependency-license-review checks that no license-incompatible dependencies have been introduced.
-  # This action is not intended to do a vulnerability check since that is handled by a separate action.
-  dependency-license-review:
-    runs-on: ubuntu-latest
-    if: github.ref != 'refs/heads/main' && github.actor != 'dependabot[bot]'
-    steps:
-      - name: "Checkout Repository"
-        uses: actions/checkout@v4
-      - name: "Dependency Review"
-        id: review
-        uses: actions/dependency-review-action@v4.3.2
-        with:
-          allow-licenses: Apache-2.0, 0BSD, BSD-2-Clause, BSD-3-Clause, CC0-1.0, ISC, MIT, MIT-0, MPL-2.0, OFL-1.1, BSD-3-Clause-Clear
-          allow-dependencies-licenses: "pkg:golang/github.com/coder/wgtunnel@0.1.13-0.20240522110300-ade90dfb2da0, pkg:npm/pako@1.0.11, pkg:npm/caniuse-lite@1.0.30001639, pkg:githubactions/alwaysmeticulous/report-diffs-action/cloud-compute"
-          license-check: true
-          vulnerability-check: false
-      - name: "Report"
-        # make sure this step runs even if the previous failed
-        if: always()
-        shell: bash
-        env:
-          VULNERABLE_CHANGES: ${{ steps.review.outputs.invalid-license-changes }}
-        run: |
-          fields=( "unlicensed" "unresolved" "forbidden" )
-
-          # This is unfortunate that we have to do this but the action does not support failing on
-          # an unknown license. The unknown dependency could easily have a GPL license which
-          # would be problematic for us.
-          # Track https://github.com/actions/dependency-review-action/issues/672 for when
-          # we can remove this brittle workaround.
-          for field in "${fields[@]}"; do
-            # Use jq to check if the array is not empty
-            if [[ $(echo "$VULNERABLE_CHANGES" | jq ".${field} | length") -ne 0 ]]; then
-              echo "Invalid or unknown licenses detected, contact @sreya to ensure your added dependency falls under one of our allowed licenses."
-              echo "$VULNERABLE_CHANGES" | jq
-              exit 1
-            fi
-          done
-          echo "No incompatible licenses detected"
diff --git a/agent/agent.go b/agent/agent.go
@@ -588,10 +588,12 @@ func (a *agent) reportMetadata(ctx context.Context, conn drpc.Conn) error {
 			updatedMetadata[mr.key] = mr.result
 			continue
 		case err := <-reportError:
-			a.logger.Debug(ctx, "batch update metadata complete", slog.Error(err))
+			logMsg := "batch update metadata complete"
 			if err != nil {
+				a.logger.Debug(ctx, logMsg, slog.Error(err))
 				return xerrors.Errorf("failed to report metadata: %w", err)
 			}
+			a.logger.Debug(ctx, logMsg)
 			reportInFlight = false
 		case <-report:
 			if len(updatedMetadata) == 0 {

diff --git a/cli/notifications_test.go b/cli/notifications_test.go
@@ -16,6 +16,16 @@ import (
 	"github.com/coder/coder/v2/testutil"
 )
 
+func createOpts(t *testing.T) *coderdtest.Options {
+	t.Helper()
+
+	dt := coderdtest.DeploymentValues(t)
+	dt.Experiments = []string{string(codersdk.ExperimentNotifications)}
+	return &coderdtest.Options{
+		DeploymentValues: dt,
+	}
+}
+
 func TestNotifications(t *testing.T) {
 	t.Parallel()
 
@@ -42,7 +52,7 @@ func TestNotifications(t *testing.T) {
 			t.Parallel()
 
 			// given
-			ownerClient, db := coderdtest.NewWithDatabase(t, nil)
+			ownerClient, db := coderdtest.NewWithDatabase(t, createOpts(t))
 			_ = coderdtest.CreateFirstUser(t, ownerClient)
 
 			// when
@@ -72,7 +82,7 @@ func TestPauseNotifications_RegularUser(t *testing.T) {
 	t.Parallel()
 
 	// given
-	ownerClient, db := coderdtest.NewWithDatabase(t, nil)
+	ownerClient, db := coderdtest.NewWithDatabase(t, createOpts(t))
 	owner := coderdtest.CreateFirstUser(t, ownerClient)
 	anotherClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
 
@@ -87,7 +97,7 @@ func TestPauseNotifications_RegularUser(t *testing.T) {
 	require.Error(t, err)
 	require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error")
 	assert.Equal(t, http.StatusForbidden, sdkError.StatusCode())
-	assert.Contains(t, sdkError.Message, "Insufficient permissions to update notifications settings.")
+	assert.Contains(t, sdkError.Message, "Forbidden.")
 
 	// then
 	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)

diff --git a/cli/organizationmembers_test.go b/cli/organizationmembers_test.go
@@ -34,49 +34,3 @@ func TestListOrganizationMembers(t *testing.T) {
 		require.Contains(t, buf.String(), owner.UserID.String())
 	})
 }
-
-func TestRemoveOrganizationMembers(t *testing.T) {
-	t.Parallel()
-
-	t.Run("OK", func(t *testing.T) {
-		t.Parallel()
-
-		ownerClient := coderdtest.New(t, &coderdtest.Options{})
-		owner := coderdtest.CreateFirstUser(t, ownerClient)
-		orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))
-		_, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
-
-		ctx := testutil.Context(t, testutil.WaitMedium)
-
-		inv, root := clitest.New(t, "organization", "members", "remove", "-O", owner.OrganizationID.String(), user.Username)
-		clitest.SetupConfig(t, orgAdminClient, root)
-
-		buf := new(bytes.Buffer)
-		inv.Stdout = buf
-		err := inv.WithContext(ctx).Run()
-		require.NoError(t, err)
-
-		members, err := orgAdminClient.OrganizationMembers(ctx, owner.OrganizationID)
-		require.NoError(t, err)
-
-		require.Len(t, members, 2)
-	})
-
-	t.Run("UserNotExists", func(t *testing.T) {
-		t.Parallel()
-
-		ownerClient := coderdtest.New(t, &coderdtest.Options{})
-		owner := coderdtest.CreateFirstUser(t, ownerClient)
-		orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))
-
-		ctx := testutil.Context(t, testutil.WaitMedium)
-
-		inv, root := clitest.New(t, "organization", "members", "remove", "-O", owner.OrganizationID.String(), "random_name")
-		clitest.SetupConfig(t, orgAdminClient, root)
-
-		buf := new(bytes.Buffer)
-		inv.Stdout = buf
-		err := inv.WithContext(ctx).Run()
-		require.ErrorContains(t, err, "must be an existing uuid or username")
-	})
-}
diff --git a/cli/organizationroles.go b/cli/organizationroles.go
@@ -203,7 +203,7 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 				// Do not actually post
 				updated = customRole
 			} else {
-				updated, err = client.PatchOrganizationRole(ctx, org.ID, customRole)
+				updated, err = client.PatchOrganizationRole(ctx, customRole)
 				if err != nil {
 					return xerrors.Errorf("patch role: %w", err)
 				}

diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -22,7 +22,8 @@ OPTIONS:
       --cache-dir string, $CODER_CACHE_DIRECTORY (default: [cache dir])
           The directory to cache temporary files. If unspecified and
           $CACHE_DIRECTORY is set, it will be used for compatibility with
-          systemd.
+          systemd. This directory is NOT safe to be configured as a shared
+          directory across coderd/provisionerd replicas.
 
       --disable-owner-workspace-access bool, $CODER_DISABLE_OWNER_WORKSPACE_ACCESS
           Remove the permission for the 'owner' role to have workspace execution

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -414,7 +414,8 @@ updateCheck: false
 # (default: <unset>, type: bool)
 enableSwagger: false
 # The directory to cache temporary files. If unspecified and $CACHE_DIRECTORY is
-# set, it will be used for compatibility with systemd.
+# set, it will be used for compatibility with systemd. This directory is NOT safe
+# to be configured as a shared directory across coderd/provisionerd replicas.
 # (default: [cache dir], type: string)
 cacheDir: [cache dir]
 # Controls whether data will be stored in an in-memory database.