Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use self-signed certificates in --no-tunnel mode #4422

Closed
wants to merge 14 commits into from

Conversation

shauns
Copy link
Contributor

@shauns shauns commented Sep 9, 2024

WHY are these changes introduced?

Changes --no-tunnel to produce https://localhost... URLs, which are more widely compatible with the rest of the Shopify platform, making that mode a compelling alternative to using a tunnel.

  • If SHOPIFY_CLI_MKCERT_BINARY is set, use the binary at that path
  • Otherwise, look for <app directory>/.shopify/mkcert in the app's codebase
  • Otherwise, prefer the system mkcert if installed (via which)
  • Download mkcert from github if needed, and place it in the app directory as above

dev is not typically used in CI/CD, but if desired, the use of the environment variable is probably best.

The certificates generated are localhost.pem and localhost-key.pem in <app directory>/.shopify.

Almost always, adding the mkcert binary and the .pem files to .gitignore is desirable. We should probably update our app templates accordingly.

WHAT is this pull request doing?

  • Adds new utilities for managing github downloads and the mkcert binary
  • Runs mkcert to generate certificates if running dev in --no-tunnel mode

How to test your changes?

Run dev with --no-tunnel. You can also use the --reset-mkcert option to get to a pristine state.

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor

github-actions bot commented Sep 9, 2024

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

Copy link
Contributor

github-actions bot commented Sep 9, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.12% (+0.11% 🔼)
8445/11550
🟡 Branches
69.57% (+0.01% 🔼)
4115/5915
🟡 Functions
71.96% (+0.21% 🔼)
2189/3042
🟡 Lines
73.44% (+0.09% 🔼)
7991/10881
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟡
... / mkcert.ts
77.78% 60.71% 75% 76.74%
🟢
... / test-with-temp-dir.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟡
... / dev.ts
74.47% (-1.62% 🔻)
65%
72.22% (-4.25% 🔻)
70.73% (-1.77% 🔻)
🔴
... / dev.ts
9.48% (-0.25% 🔻)
1.52% (-0.05% 🔻)
15.79%
9.35% (-0.27% 🔻)
🟢
... / github.ts
91.11% (+4.9% 🔼)
85.71% (-2.52% 🔻)
87.5% (+7.5% 🔼)
91.11% (+4.9% 🔼)

Test suite run success

1900 tests passing in 861 suites.

Report generated by 🧪jest coverage report action from cc4b21a

Copy link
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/public/node/testing/test-with-temp-dir.d.ts
import { TestAPI } from 'vitest';
interface TempDirFixture {
    tempDir: string;
}
/**
 * Vitest fixture providing the test with a temporary directory to work in.
 */
export declare const testWithTempDir: TestAPI<TempDirFixture>;
export {};

Existing type declarations

packages/cli-kit/dist/public/node/github.d.ts
@@ -47,4 +47,5 @@ export interface GithubRepositoryReference {
  * @param reference - A GitHub repository URL (e.g. https://github.com/Shopify/cli/blob/main/package.json)
  */
 export declare function parseGitHubRepositoryReference(reference: string): GithubRepositoryReference;
+export declare function downloadGitHubRelease(repo: string, version: string, assetName: string, targetPath: string): Promise<void>;
 export {};
\ No newline at end of file

@shauns
Copy link
Contributor Author

shauns commented Sep 19, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @shauns! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@shauns
Copy link
Contributor Author

shauns commented Sep 19, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @shauns! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

Copy link
Contributor

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.
→ If there's no activity within a week, then a bot will automatically close this.
Thanks for helping to improve Shopify's dev tooling and experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant