[Bug]: Unexpect update on Yarn.lock during auto version upgrade

[noCharger]

Describe the bug

There are couple of PRs create on dashboards-search-relevance with title "[AUTO] Increment version" which didn't change the version but the yarn.lock file. Here's some dive deep:

1. The most recent change related toyarn osd bootstrap is OSD version increment tmp fix #3509
2. yarn osd bootstrap is required to run node ../../scripts/plugin_helpers version --sync legacy
3. yarn osd bootstrap results in the yarn.lock file changed.

To reproduce

1. Download OSD, install dashboards-search-relevance
2. Run node ../../scripts/plugin_helpers version --sync legacy directly

dashboards-search-relevance % node ../../scripts/plugin_helpers version --sync legacy
internal/modules/cjs/loader.js:905
  throw err;
  ^

Error: Cannot find module 'require-in-the-middle'
Require stack:
- /Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/harden/child_process.js
- /Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/harden/index.js
- /Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/no_transpilation.js
- /Volumes/workplace/OpenSearch-Dashboards/scripts/plugin_helpers.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
    at Function.Module._load (internal/modules/cjs/loader.js:746:27)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:101:18)
    at Object.<anonymous> (/Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/harden/child_process.js:31:12)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/harden/child_process.js',
    '/Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/harden/index.js',
    '/Volumes/workplace/OpenSearch-Dashboards/src/setup_node_env/no_transpilation.js',
    '/Volumes/workplace/OpenSearch-Dashboards/scripts/plugin_helpers.js'
  ]
}

3. Run yarn osd bootstrap results in the yarn.lock file changed.

Expected behavior

Open questions:

1. Is dashboards-search-relevance the only plugin impacted? Is it possible to opt out the auto version increment for the short term?
2. For the long term, shall we evaluate the yarn.lock change during the auto version increment or just exclude the changes in this file?

Screenshots

If applicable, add screenshots to help explain your problem.

Host / Environment

No response

Additional context

No response

Relevant log output

No response

Acceptance Criteria

• Update the OSD version increment workflow to exclude the Yarn.lock file during the PR creation stage.

[bbarani]

@AMoo-Miki Do you have any inputs here?

[AMoo-Miki]

The yarn.lock file of a plugin reflects the packages that the plugin is dependent on; it does not include OSD's deps. The CI that bumps the versions and raises the PRs is just being helpful in updating the lockfile of the plugins to match what it really would be if it is built.

For example, [email protected] is dependent on tough-cookie@~2.5.0 via its dependency on [email protected]. When the plugin is built, tough-cookie@~2.5.0 will be installed even though the lockfile references tough-cookie@^4.1.3 because the version ranges do not match. The ^4.1.3 is just an extra info that would be removed from the lockfile.

On one hand, I like that the CI is being helpful and is automating the task that repo maintainers should be doing but on the other hand, I don't like the PR doing something other that what it claims in its title.

So:

• Unexpected update?
  • Yes, it is unexpected because the PR says it will just bump the version
• Is the change valid?
  • Yes. The repo clearly has a wrong entry in the lockfile.
• What should the plugin do?
  • Make sure their lockfile matches reality. In this case, they are missing a resolution (example).

[gaiksaya]

Thanks for the findings @AMoo-Miki . I see two options here:

1. We can ignore the yarn.lock file while creating the PR (that way the PR is what the title says)
2. Its upon the maintainers to either accept or decline the change. PR titles can be managed by the maintainers (so just change it?) or decline the PR and create their own to bump the version.

WDYT?

cc: @mingshl

[AMoo-Miki]

Sayali I love automation and what this workflow is doing is pointing out a problem with the repos that they might otherwise not catch. The workflow, in its current state, will include the lockfile only and only if the existing lockfile is invalid.

The best solution, now that we have this much done, would be to:

1. raise the [AUTO] Increment version PR only with the package.json, if it changes as a result of this workflow.
2. raise an issue if the lockfile was changed during the execution of this workflow, stating that the lockfile is outdated and including the diff in the body of the issue.

There is another option too: Simply change the title of the PR to say [AUTO] Preparations for x.x.x release, or [AUTO] Version and lockfile sync for release x.x.x. The only downside of this option is that maintainers might not realize the meaning of the changes to the lockfile and might dismiss it but an issue will grab their attention.

[noCharger]

Thank you for your insightful comments, @AMoo-Miki. I like the idea of separating version increment and lockfile check. Also upgrades to plugin side dependencies should change the package.json. DSR plugin should come up with the fix and maybe revert opensearch-project/dashboards-search-relevance#239.

[gaiksaya]

I believe from version increment point of view, raising issues for missed changes is out of scope of this effort. We can proceed with

1. raise the [AUTO] Increment version PR only with the package.json, if it changes as a result of this workflow.

Let me know if that is okay with you @bbarani @prudhvigodithi
Thanks!

[macohen]

What's the next step on this? We still have conflicts on the yarn.lock that I think we need to merge manually to ensure we stay on the latest and can build for 2.10. In the future, we should not receive updates to yarn.lock. Instead package.json should receive updates for auto version upgrade. Is that right?

[gaiksaya]

Hi @prudhvigodithi
Looking at https://github.com/peter-evans/create-pull-request I did not find anything to exclude or ignore a file while creating a PR. I believe resetting the yarn.lock before we call that action is the solution here?
WDYT?

[prudhvigodithi]

Circling back on this issue, based on @AMoo-Miki feedback what we can do is to just raise a PR with only opensearch_dashboards.json and package.json as they are the only core files that dictate the version increment.

          add-paths: |
              opensearch_dashboards.json
              package.json

[prudhvigodithi]

Here is the PR that should fix this bug to raise a version increment PR with only package.json and opensearch_dashboards.json files.
Thank you

[prudhvigodithi]

The PR was merged and works as expected, it closed the PR opensearch-project/dashboards-search-relevance#281 for version increment 2.9.0.0 as it dint identify package.json or opensearch_dashboards.json files (ignores all other files like yarn.lock).

[prudhvigodithi]

Closing this Issue, @noCharger please re-open if required.
Thank you