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

fix(deployment): SDK publishing failure due to missing or incorrect (#30246) #30247

Merged
merged 14 commits into from
Oct 8, 2024

Conversation

dcolina
Copy link
Contributor

@dcolina dcolina commented Oct 4, 2024

Proposed Changes

  • This PR fixes an authentication issue in the SDK publishing process by ensuring the correct token is provided. The previous configuration caused authentication failures (npm ERR! need auth) due to a missing or incorrect token.

Additional Info

This PR resolves #30246 (SDK publishing failure due to missing or incorrect).

This PR fixes: #30246

@dcolina dcolina requested a review from a team as a code owner October 4, 2024 11:07
@dcolina dcolina linked an issue Oct 4, 2024 that may be closed by this pull request
3 tasks
@dcolina dcolina marked this pull request as draft October 4, 2024 14:29
@dcolina dcolina marked this pull request as ready for review October 8, 2024 09:08
…f github.com:dotCMS/core into issue-30246-sdk-publishing-failure-due-to-missing-or
@spbolton
Copy link
Contributor

spbolton commented Oct 8, 2024

I am assuming here we need to have the node version here be independent from the node version we build with. If not we should be relying on the node and npm managed from maven with core-web. The prepare maven action has a "requires-node" option that sets up maven. I am just not sure if we need to do something different here due to it being the sdk.

@dcolina
Copy link
Contributor Author

dcolina commented Oct 8, 2024

I am assuming here we need to have the node version here be independent from the node version we build with. If not we should be relying on the node and npm managed from maven with core-web. The prepare maven action has a "requires-node" option that sets up maven. I am just not sure if we need to do something different here due to it being the sdk.

Done. @spbolton Now node and npm are managed through the prepare maven action.

@dcolina dcolina enabled auto-merge October 8, 2024 18:02
@dcolina dcolina disabled auto-merge October 8, 2024 18:24
@spbolton
Copy link
Contributor

spbolton commented Oct 8, 2024

If we only need to install the right version of node and not generating other maven artifacts.
then the maven args could be
‘process-resources -pl :dotcms-core-web -am’
The (-am) also make is to ensure it picks up the parent poms.
The frontend-maven plugin runs in the generate-resources phase and the maven-antrun-plugin creates/updates the .nvmrc file in process-resources phase, and you do not need to go any further e.g. the next phase is compile so it will stop before compiling anything. you could still add clean, but there is no need really as there is nothing to clean really.
The phases are here for reference https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#default-lifecycle
Now in theory the node and yarn binaries should be still in the cache from previous build, the maven-job action restores this cache, but there is no harm in re-running here.
What I think we are not doing is setting the $PATH in the runner for yarn and node to the versions we restore into the installs folder in the repo root.
just running yarn etc. will likely not use the version maven downloaded or restored from the cache.

We have code in our pre-commit hook that sets up node and yarn from these versions and we can do similar or wrap in an action. Just realized that the mvnw validate in our pre-commit hook is not enough to ensure the yarn and node are downloaded. Most of the time this is not a problem because a previous build will have created these files, but we will need to fix this.

if ! ./mvnw validate -pl :dotcms-core --am -q; then
    print_color "$RED" "Failed to run './mvnw validate -pl :dotcms-core --am'"
    print_color "$YELLOW" "Please run the following command to see the detailed output:"
    printf "./mvnw validate -pl :dotcms-core --am\n"
    exit 1
else
    printf "Completed Maven init\n"
fi

# Update PATH to include project-specific Node.js and Yarn
export PATH="${root_dir}/installs/node:${root_dir}/installs/node/yarn/dist/bin:$PATH"

# Verify that node and yarn are accessible
if ! command -v node > /dev/null 2>&1 || ! command -v yarn > /dev/null 2>&1; then
    print_color "$RED" "Error: Node.js or Yarn not found in the project directory."
    print_color "$YELLOW" "Please ensure they are installed in ${root_dir}/installs/node"
    exit 1
fi

@nollymar nollymar added this pull request to the merge queue Oct 8, 2024
@dcolina dcolina removed this pull request from the merge queue due to a manual request Oct 8, 2024
@dcolina dcolina enabled auto-merge October 8, 2024 19:26
@dcolina
Copy link
Contributor Author

dcolina commented Oct 8, 2024

If we only need to install the right version of node and not generating other maven artifacts. then the maven args could be ‘process-resources -pl :dotcms-core-web -am’ The (-am) also make is to ensure it picks up the parent poms. The frontend-maven plugin runs in the generate-resources phase and the maven-antrun-plugin creates/updates the .nvmrc file in process-resources phase, and you do not need to go any further e.g. the next phase is compile so it will stop before compiling anything. you could still add clean, but there is no need really as there is nothing to clean really. The phases are here for reference https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#default-lifecycle Now in theory the node and yarn binaries should be still in the cache from previous build, the maven-job action restores this cache, but there is no harm in re-running here. What I think we are not doing is setting the $PATH in the runner for yarn and node to the versions we restore into the installs folder in the repo root. just running yarn etc. will likely not use the version maven downloaded or restored from the cache.

Done. @spbolton This has been updated setting up the correct path to the restored node and yarn versions.

We have code in our pre-commit hook that sets up node and yarn from these versions and we can do similar or wrap in an action. Just realized that the mvnw validate in our pre-commit hook is not enough to ensure the yarn and node are downloaded. Most of the time this is not a problem because a previous build will have created these files, but we will need to fix this.

if ! ./mvnw validate -pl :dotcms-core --am -q; then
    print_color "$RED" "Failed to run './mvnw validate -pl :dotcms-core --am'"
    print_color "$YELLOW" "Please run the following command to see the detailed output:"
    printf "./mvnw validate -pl :dotcms-core --am\n"
    exit 1
else
    printf "Completed Maven init\n"
fi

# Update PATH to include project-specific Node.js and Yarn
export PATH="${root_dir}/installs/node:${root_dir}/installs/node/yarn/dist/bin:$PATH"

# Verify that node and yarn are accessible
if ! command -v node > /dev/null 2>&1 || ! command -v yarn > /dev/null 2>&1; then
    print_color "$RED" "Error: Node.js or Yarn not found in the project directory."
    print_color "$YELLOW" "Please ensure they are installed in ${root_dir}/installs/node"
    exit 1
fi

This part it will faced in another card.

Copy link

@dcolina dcolina added this pull request to the merge queue Oct 8, 2024
Merged via the queue into main with commit 130efbc Oct 8, 2024
35 checks passed
@dcolina dcolina deleted the issue-30246-sdk-publishing-failure-due-to-missing-or branch October 8, 2024 22:47
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.

SDK publishing failure due to missing or incorrect authentication token
5 participants