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

Package Release: Update usage of NPM auth token #44215

Merged
merged 11 commits into from
Feb 1, 2024
Merged

Conversation

psealock
Copy link
Contributor

@psealock psealock commented Jan 31, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

NPM auth token was being incorrectly used, this PR fixes it according to NPM docs.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. I kicked off the Release workflow on a package that was ready to publish using this branch. See the workflow succeed here: https://github.com/woocommerce/woocommerce/actions/runs/7722378461/job/21050460230
  2. Expand the "Execute script" and confirm the script that was executed referencing this branch (fix/npm-auth-token-use) and the @woocommerce/create-woo-extension package.
  3. Note also the 1.1.0 version successfully published at the end of the section's logs
  4. Confirm the package was indeed updated on NPM: https://www.npmjs.com/package/@woocommerce/create-woo-extension

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@psealock psealock requested a review from a team January 31, 2024 07:13
Copy link
Contributor

github-actions bot commented Jan 31, 2024

Hi @rrennick,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

Copy link
Contributor

github-actions bot commented Jan 31, 2024

Test Results Summary

Commit SHA: 41404fb

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 37s
E2E Tests287002103086m 53s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

Copy link
Contributor

@rrennick rrennick left a comment

Choose a reason for hiding this comment

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

Nice work. Thanks @psealock

@rrennick rrennick self-requested a review January 31, 2024 18:27
Copy link
Contributor

@rrennick rrennick left a comment

Choose a reason for hiding this comment

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

@psealock This is failing CI because the changes output a warning before the project graph:

WARN  Issue while reading "/Users/ronrennick/Sites/wc/wp-content/woocommerce/.npmrc". Failed to replace env in config: ${NODE_AUTH_TOKEN}

Locally for me, NODE_AUTH_TOKEN is not set when running ci-jobs.

@psealock
Copy link
Contributor Author

psealock commented Jan 31, 2024

@ObliviousHarmony @rrennick I've changed this up such that authentication is set in the action itself instead of inside .npmrc. Without a package ready to publish I used an npm whoami call to test authentication. Below are updated test instructions.

  1. Locally check to make sure other scripts don't attempt to set the token, pnpm utils ci-jobs should succeed without error.
  2. See this run of the Publish action. Instead of running the publishing script, I call npm whoami which fails unless authentication works. Check the "Execute script" logs to confirm the output is correct, ~woocommerce.

# run: ./tools/package-release/bin/dev publish ${{ github.event.inputs.packages }} --branch=${{ github.ref_name }} ${{ ( github.event.inputs.dry_run == 'true' && '--dry-run' ) || '' }} --skip-install
run: |
npm config set '//registry.npmjs.org/:_authToken' "${NODE_AUTH_TOKEN}"
npm whoami

  1. I reverted that test and replaced the publish script call, see that this makes sense.

Copy link
Contributor

@rrennick rrennick left a comment

Choose a reason for hiding this comment

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

This tested great @psealock

@rrennick rrennick merged commit 33c2c0c into trunk Feb 1, 2024
29 checks passed
@rrennick rrennick deleted the fix/npm-auth-token-use branch February 1, 2024 17:23
thealexandrelara pushed a commit that referenced this pull request Feb 1, 2024
* update .npmrc

* try project specific .npmrc

* try whoami

* try package .npmrc

* whitespace

* try in tool

* try adding line

* set auth explicitly

* test whoami

* does this work?

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

Successfully merging this pull request may close these issues.

None yet

2 participants