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

SQC-352 SQC-353 Create cert upload command for client side mtls certificate/CA chain certificates #7466

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ltadrian
Copy link

@Ltadrian Ltadrian commented Dec 5, 2024

Create cert upload command for client side mtls certificate/CA chain certificates

Fixes #[insert GH or internal issue link(s)].

  • Allows the capability to upload custom CA certificates and client mTLS certificates. This is a lot similar to mtls-certificate command but our goal was to avoid confusion with workers support. This custom CA certificates upload is not a supported binding for workers. It will be available as a Hyperdrive binding very soon.

Internal JIRA
SQC-352
SQC-353


@Ltadrian Ltadrian requested a review from a team as a code owner December 5, 2024 23:25
Copy link

changeset-bot bot commented Dec 5, 2024

🦋 Changeset detected

Latest commit: c396657

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/wrangler/src/cert/cli.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/cert.test.ts Outdated Show resolved Hide resolved
@lrapoport-cf
Copy link
Contributor

hi @Ltadrian :) we recently introduced a new createCommand utility to be used when new commands are being added. can you please review https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/CONTRIBUTING.md and update this PR accordingly? thank you!

@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch 4 times, most recently from 15327ba to 03af501 Compare December 10, 2024 18:58
@Ltadrian
Copy link
Author

hi @Ltadrian :) we recently introduced a new createCommand utility to be used when new commands are being added. can you please review https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/CONTRIBUTING.md and update this PR accordingly? thank you!

this has been updated now, thank you 😄

@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch 4 times, most recently from e3d8b3d to 4450a1b Compare December 31, 2024 16:02
Copy link
Contributor

github-actions bot commented Dec 31, 2024

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-wrangler-7466

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7466/npm-package-wrangler-7466

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-wrangler-7466 dev path/to/script.js
Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-workers-bindings-extension-7466 -O ./cloudflare-workers-bindings-extension.0.0.0-vdf2e07461.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vdf2e07461.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-create-cloudflare-7466 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-kv-asset-handler-7466

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-miniflare-7466

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-pages-shared-7466

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-unenv-preset-7466

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-vitest-pool-workers-7466

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-workers-editor-shared-7466

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-workers-shared-7466

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12800697590/npm-package-cloudflare-workflows-shared-7466

Note that these links will no longer work once the GitHub Actions artifact expires.


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241230.2
workerd 1.20241230.0 1.20241230.0
workerd --version 1.20241230.0 2024-12-30

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@emily-shen emily-shen added the e2e Run e2e tests on a PR label Jan 2, 2025
Copy link
Contributor

@emily-shen emily-shen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would also be great to add some e2e tests :)

packages/wrangler/src/__tests__/cert.test.ts Outdated Show resolved Hide resolved
.changeset/hip-cameras-yawn.md Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
Comment on lines +114 to +132
typeof certs === "undefined"
? [
{
id: "1234",
name: "cert one",
certificates: "BEGIN CERTIFICATE...",
issuer: "example.com...",
uploaded_on: now.toISOString(),
expires_on: oneYearLater.toISOString(),
},
{
id: "5678",
name: "cert two",
certificates: "BEGIN CERTIFICATE...",
issuer: "example.com...",
uploaded_on: now.toISOString(),
expires_on: oneYearLater.toISOString(),
},
]
: certs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do
certs ?? [... whatever you want to return if undefined]

packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
packages/wrangler/src/api/mtls-certificate.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch 2 times, most recently from 4f0a3b6 to 9356364 Compare January 2, 2025 17:35
@Ltadrian
Copy link
Author

Ltadrian commented Jan 2, 2025

would also be great to add some e2e tests :)

is that required?

@emily-shen
Copy link
Contributor

would also be great to add some e2e tests :)

is that required?

yes, all new commands/features should have some happy-path e2e's before release :)

@emily-shen emily-shen removed the e2e Run e2e tests on a PR label Jan 3, 2025
@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch from 9356364 to 44fbd6f Compare January 14, 2025 03:28
@workers-devprod workers-devprod added the e2e Run e2e tests on a PR label Jan 14, 2025
@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch 2 times, most recently from 6b67e53 to 9c7342b Compare January 14, 2025 03:52
@emily-shen emily-shen removed the e2e Run e2e tests on a PR label Jan 14, 2025
Copy link
Contributor

@emily-shen emily-shen Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to generate a new cert and key each time? Just concerned about simultaneous CI runs interfering with each other.

Other than that, lgtm :))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been updated to generate certs

@workers-devprod workers-devprod added the e2e Run e2e tests on a PR label Jan 14, 2025
@emily-shen emily-shen removed the e2e Run e2e tests on a PR label Jan 15, 2025
@Ltadrian Ltadrian force-pushed the ltadrian/cert-upload-mtls branch from 545288b to 0b9120a Compare January 15, 2025 15:46
@workers-devprod workers-devprod added the e2e Run e2e tests on a PR label Jan 15, 2025
Copy link
Contributor

@mtlemilio mtlemilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor observations.

packages/wrangler/src/api/mtls-certificate.ts Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Outdated Show resolved Hide resolved
packages/wrangler/src/cert/cert.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Run e2e tests on a PR
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

5 participants