Add single version loose param for building plugins #4468

derek-ho · 2024-02-21T14:11:59Z

Description

Coming from: opensearch-project/OpenSearch-Dashboards#5675, this PR adds the flag to have loose single-version dependencies. Ideally this should not be enforced for devDependencies, but should unblock the build.

Why these changes are required?

OSD core added a dependency to cypress 9.5.4 (mismatching with the version in this repo) here: https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5725/files#r1494915750, leading to error at bootstrap.

What is the old behavior before changes and new behavior after changes?

Old behavior is that the bootstrap process would fail. OSD core added a dependency to cypress 9.5.4 in this PR: https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5725/files#r1494915750. Since we need cypress 13 in order to run our test suites (including cross-origin support), bootstrapping the plugin with OSD fails since there is a version mismatch. With this new flag, this inconsistency is ignored and the CI is green.

Issues Resolved

Related to: opensearch-project/security-dashboards-plugin#1786. However that issue is coming from autocuts. We need opensearch-build repo to take in this change to close the autocut.

Testing

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Derek Ho <[email protected]>

derek-ho · 2024-02-21T14:13:46Z

@peterzhuamazon @prudhvigodithi can you folks take a look at this? I see autocuts in several repos that should be fixed by this change.

derek-ho · 2024-02-21T14:14:13Z

#4345

codecov · 2024-02-21T14:16:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.19%. Comparing base (d7a9e4c) to head (04a994e).
Report is 223 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4468      +/-   ##
==========================================
+ Coverage   91.99%   92.19%   +0.20%     
==========================================
  Files         192      192              
  Lines        6284     6292       +8     
==========================================
+ Hits         5781     5801      +20     
+ Misses        503      491      -12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

peterzhuamazon

In build we have already decided to not use this param, due to us already have a way to cleanup previous plugin build leftovers.

ruanyl · 2024-02-26T05:40:16Z

In build we have already decided to not use this param, due to us already have a way to cleanup previous plugin build leftovers.

It could be that the single version check failed due to the version difference between OSD core and the plugin. I suppose we still need the loose flag.

Signed-off-by: Derek Ho <[email protected]>

prudhvigodithi · 2024-02-28T19:53:16Z

@derek-ho in .github/workflows/osd-increment-plugin-versions.yml there are two place the yarn osd bootstrap called, can you update both ?

Signed-off-by: Derek Ho <[email protected]>

derek-ho · 2024-02-28T20:06:06Z

@derek-ho in .github/workflows/osd-increment-plugin-versions.yml there are two place the yarn osd bootstrap called, can you update both ?

Done - missed that one, thanks! @prudhvigodithi lint checker is timing out, seems flaky, can you re-run it?

peterzhuamazon · 2024-02-29T18:00:49Z

scripts/default/opensearch-dashboards/build.sh

@@ -98,7 +98,7 @@ mkdir -p $OUTPUT/plugins
 cp -r ../$PLUGIN_NAME/ ../OpenSearch-Dashboards/plugins
 echo "BUILD MODULES FOR $PLUGIN_NAME"
 CURR_DIR=`pwd`
-cd ../OpenSearch-Dashboards; eval $NVM_CMD; yarn osd bootstrap
+cd ../OpenSearch-Dashboards; eval $NVM_CMD; yarn osd bootstrap --single-version=loose


This is not fully applicable due to this will affect 1.x.

@peterzhuamazon what do you suggest here, should we add a version check here to make sure we only run this above a specific version?

@peterzhuamazon Any idea how to proceed here? Looks like many plugins are failing for 2.13.0 because of this.

prudhvigodithi · 2024-03-05T21:27:59Z

@derek-ho can you please fix the failing checks?
also I'm good with this change, @peterzhuamazon @bbarani can you add your thoughts ?
Thanks

derek-ho · 2024-03-06T19:25:08Z

@derek-ho can you please fix the failing checks? also I'm good with this change, @peterzhuamazon @bbarani can you add your thoughts ? Thanks

I think the lint check failing is not due to my changes - it is complaining about release notes, which I did not touch. Can you re-run the check again? Please let me know what else needs to happen to get this in - it will resolve several autocuts, thanks!

peterzhuamazon · 2024-03-15T22:02:39Z

Will discuss with @derek-ho to resolve this.

peterzhuamazon · 2024-03-18T22:53:24Z

Waiting to see if same issues showing up in 2.13, seems fine now according to @derek-ho.

peterzhuamazon · 2024-03-26T17:14:19Z

Seems running fine in 2.13 version.
Close for now.

Thanks.

ruanyl · 2024-04-23T09:30:11Z

The builds are still failing due to single version check for security dashboards plugin, CI logs here

Error log shows it failed on single version check for cypress

ERROR [single_version_dependencies] Multiple version ranges for the same dependency
      were found declared across different package.json files. Please consolidate
      those to match across all package.json files. Different versions for the
      same dependency is not supported.

      If you have questions about this please reach out to the operations team.

      The conflicting dependencies are:

        cypress
          9.5.4 => opensearch-dashboards
          ^13.6.0 => opensearch-security-dashboards

The reason why it succeed on release branch is opensearch-dashboards has cypress as its dependency only on main but not on release branch 2.x.

derek-ho · 2024-05-03T15:29:24Z

The builds are still failing due to single version check for security dashboards plugin, CI logs here

Error log shows it failed on single version check for cypress
ERROR [single_version_dependencies] Multiple version ranges for the same dependency
      were found declared across different package.json files. Please consolidate
      those to match across all package.json files. Different versions for the
      same dependency is not supported.

      If you have questions about this please reach out to the operations team.

      The conflicting dependencies are:

        cypress
          9.5.4 => opensearch-dashboards
          ^13.6.0 => opensearch-security-dashboards
The reason why it succeed on release branch is opensearch-dashboards has cypress as its dependency only on main but not on release branch 2.x.

@peterzhuamazon @prudhvigodithi can you help us with this?

peterzhuamazon · 2024-05-03T22:22:06Z

Seems like RC2 is already out.
#4562 (comment)

Please re-open if you still need this.

Thanks.

ruanyl · 2024-05-06T09:00:47Z

Thanks @peterzhuamazon, the point is it succeed on release branch(2.x) but not on main branch.

derek-ho · 2024-06-20T15:32:32Z

@peterzhuamazon I think we need to get this merged to get security building on the main branch again, can you help out? This is my analysis showing that we need this PR merged:
This commit in OSD core introduced cypress 9.5.4 (while security uses 13+).

The latest docker image of 3.0 is using the highlighted commit, which is the last day prior to the merging of the PR that introduced cypress. Thus we need to introduce the loose param to allow building of security dashboards plugin in the docker image. Can you please help us get this merged? This is blocking cypress testing in the FTR on the main branch

CC @SuZhou-Joe @cwperks

peterzhuamazon · 2024-06-20T17:31:19Z

@peterzhuamazon I think we need to get this merged to get security building on the main branch again, can you help out? This is my analysis showing that we need this PR merged: This commit in OSD core introduced cypress 9.5.4 (while security uses 13+).

The latest docker image of 3.0 is using the highlighted commit, which is the last day prior to the merging of the PR that introduced cypress. Thus we need to introduce the loose param to allow building of security dashboards plugin in the docker image. Can you please help us get this merged? This is blocking cypress testing in the FTR on the main branch

CC @SuZhou-Joe @cwperks

Maybe we should prioritize upgrading cypress instead of allowing loose paramter.
opensearch-project/opensearch-dashboards-functional-test#408

Thoughts? cc: @ruanyl

derek-ho · 2024-06-27T17:07:01Z

@peterzhuamazon @ruanyl @cwperks @SuZhou-Joe adding some more weight behind this decision: I see several autocuts across multiple plugin repos: https://github.com/search?q=org%3Aopensearch-project+%5BAUTOCUT%5D+Distribution+Build+Failed+for+*-3.0.0+is%3Aopen&type=issues. It seems like many of these were also opened around feb 16 time frame, and may also be due to incompatible cypress dependency. I think we should try to fix this ASAP to get the docker images/builds updated. Right now we are several months behind. I think bumping cypress would take some time/buy in from the core dashboards team, can we get this merged in as a temporary fix please?

cwperks · 2024-06-27T17:30:09Z

Maybe we should prioritize upgrading cypress instead of allowing loose paramter.
opensearch-project/opensearch-dashboards-functional-test#408

Thoughts? cc: @ruanyl

Since this is a dev dependency, what is the drawback to letting dashboards plugin developer teams upgrade this dependency independently?

Security needs Cypress >= 12 for cross-origin testing. If its mandated that all dashboards-plugins use the same version then it requires all dashboards plugins to upgrade before Security can get the features needed for testing SSO.

What is the effort to upgrade the version of cypress in OSD core? OSD core added cypress as a dependency 4 months ago in opensearch-project/OpenSearch-Dashboards@55443f7 which is when the dashboards plugins started failing.

ruanyl · 2024-06-28T02:04:29Z

Maybe updating cypress could suppress the current issue now, but it's just a mitigation in my opinion. It's cypress today, in the future it could be something else.

I think plugin owner should not be limited to use the same version of DEV dependencies as OSD core or other plugins, they should have the "freedom" to choose the tools that's better for them during the development as long as the dependencies are the same. And the loose flag will only skip checking the DEV dependencies as I know (@AMoo-Miki correct me if I'm wrong).

derek-ho · 2024-06-28T15:50:24Z

@peterzhuamazon thoughts?

derek-ho · 2024-07-12T16:40:38Z

(24-07-12 16:32:32) <11> [~/OpenSearch-Dashboards]  
dev-dsk-dxho-1d-8102284b % nvm use --delete-prefix v10.24.1
Now using node v10.24.1 (npm v6.14.12)

(24-07-12 16:32:47) <0> [~/OpenSearch-Dashboards]  
dev-dsk-dxho-1d-8102284b % yarn osd bootstrap --single-version=loose
yarn run v1.22.22
$ node scripts/osd bootstrap --single-version=loose
 info [opensearch-dashboards] running yarn

$ node ./preinstall_check
[1/5] Validating package.json...
[2/5] Resolving packages...
warning Resolution field "[email protected]" is incompatible with requested version "typescript@~4.8.4"
[3/5] Fetching packages...
[4/5] Linking dependencies...
warning " > @elastic/[email protected]" has incorrect peer dependency "typescript@^3.7.2".
warning " > @testing-library/[email protected]" has unmet peer dependency "react-test-renderer@>=16.9.0".
[5/5] Building fresh packages...

 succ yarn.lock analysis completed without any issues
 info [@osd/cross-platform] running [osd:bootstrap] script
 info [@osd/utility-types] running [osd:bootstrap] script
 succ [@osd/utility-types] bootstrap complete
 succ [@osd/cross-platform] bootstrap complete
 info [@osd/config-schema] running [osd:bootstrap] script
 info [@osd/std] running [osd:bootstrap] script
 succ [@osd/std] bootstrap complete
 succ [@osd/config-schema] bootstrap complete
 info [@osd/logging] running [osd:bootstrap] script
 info [@osd/utils] running [osd:bootstrap] script
 succ [@osd/logging] bootstrap complete
 succ [@osd/utils] bootstrap complete
 info [@osd/apm-config-loader] running [osd:bootstrap] script
 info [@osd/dev-utils] running [osd:bootstrap] script
 succ [@osd/apm-config-loader] bootstrap complete
 succ [@osd/dev-utils] bootstrap complete
 info [@osd/ace] running [osd:bootstrap] script
 info [@osd/analytics] running [osd:bootstrap] script
 info [@osd/config] running [osd:bootstrap] script
 info [@osd/i18n] running [osd:bootstrap] script
 succ [@osd/analytics] bootstrap complete
 info [@osd/monaco] running [osd:bootstrap] script
 succ [@osd/ace] bootstrap complete
 info [@osd/opensearch-archiver] running [osd:bootstrap] script
 succ [@osd/i18n] bootstrap complete
 info [@osd/opensearch] running [osd:bootstrap] script
 succ [@osd/config] bootstrap complete
 info [@osd/plugin-generator] running [osd:bootstrap] script
 succ [@osd/opensearch] bootstrap complete
 info [@osd/release-notes] running [osd:bootstrap] script
 succ [@osd/plugin-generator] bootstrap complete
 info [@osd/telemetry-tools] running [osd:bootstrap] script
 succ [@osd/opensearch-archiver] bootstrap complete
 info [@osd/test] running [osd:bootstrap] script
 succ [@osd/monaco] bootstrap complete
 succ [@osd/release-notes] bootstrap complete
 succ [@osd/telemetry-tools] bootstrap complete
 succ [@osd/test] bootstrap complete
 info [@osd/interpreter] running [osd:bootstrap] script
 info [@osd/ui-shared-deps] running [osd:bootstrap] script
 succ [@osd/interpreter] bootstrap complete
 succ [@osd/ui-shared-deps] bootstrap complete
 info [@osd/optimizer] running [osd:bootstrap] script
 succ [@osd/optimizer] bootstrap complete
 info [@osd/plugin-helpers] running [osd:bootstrap] script
 succ [@osd/plugin-helpers] bootstrap complete
 info [opensearch-dashboards] running [osd:bootstrap] script
 succ [opensearch-dashboards] bootstrap complete
Done in 181.95s.

(24-07-12 16:35:57) <0> [~/OpenSearch-Dashboards]  
dev-dsk-dxho-1d-8102284b % git status
On branch 1.3
Your branch is up to date with 'origin/1.3'.

nothing to commit, working tree clean

Verified that this does not throw errors on 1.3 branch, so no version logic is needed

AMoo-Miki · 2024-07-12T16:42:24Z

It's cypress today, in the future it could be something else.

You are absoloutely right! cypress, husky, lint-staged, formik, @testing-library/user-event, @types/react-plotly.js, plotly.js-dist, react-plotly.js, @types/react-test-renderer, ts-jest, cypress-multi-reporters, react-graph-vis, ... are just the ones we have had to deal with since I started taking down names. Without loose we are going to have more.

... And the loose flag will only skip checking the DEV dependencies as I know (@AMoo-Miki correct me if I'm wrong).

There are 3 things that loose does:

Doesn't complain about version range mismatch for dev deps
Doesn't complain about version range mismatch if all of them end up with the same version in lockfile
If version range mismatch manifests as different versions in lockfiles, it will use the latest one of those found in the lockfiles that satisfies all version ranges requested.

All 3 of these are completely safe.

Additionally, @derek-ho ran an experiment and found that the extra CLI parameter handed to yarn osd bootstrap doesn't cause a failure in 1.3. This indicates that we don't need special handling in 1.3 builds.

peterzhuamazon · 2024-07-12T19:52:13Z

After having a discussion with @derek-ho offline.

We cleared some confusions and proposed some new solutions:

If plugin only have a version diff due to cypress 9 vs 13, revert them to 9 if possible
For plugins that have more than one dep diff, such as reportDashboards:

        @types/dompurify
          ^3.0.5 => opensearch-dashboards
          ^2.3.3 => reports-dashboards
        cypress
          9.5.4 => opensearch-dashboards
          ^13.6.0 => reports-dashboards
        dompurify
          ^3.0.11 => opensearch-dashboards
          ^2.4.1 => reports-dashboards

Since --single-version might also touch regular dependencies, we suggest adding individual custom build script in corresponding plugin repo as scripts/build.sh, instead of changing on the default build script for all plugins.

Revert the change of default build script of OSD in this PR, and merge with only the OS/OSD incremental workflow changes.

Thanks.

Signed-off-by: Derek Ho <[email protected]>

peterzhuamazon · 2024-07-15T22:54:27Z

As we discussed this PR will only include --single-version changes in version bump workflows.
Example custom build.sh are added like this: opensearch-project/security-dashboards-plugin#2027

Thanks.

Add single version loose param

64400f5

Signed-off-by: Derek Ho <[email protected]>

derek-ho requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi, Divyaasm and tianleh as code owners February 21, 2024 14:12

github-actions bot added the valued-contributor label Feb 21, 2024

derek-ho mentioned this pull request Feb 21, 2024

[AUTOCUT] Distribution Build Failed for securityDashboards-3.0.0 opensearch-project/security-dashboards-plugin#1786

Closed

peternied mentioned this pull request Feb 21, 2024

Ignore devDependencies version mismatch in bootstrap opensearch-project/OpenSearch-Dashboards#5561

Closed

peterzhuamazon requested changes Feb 22, 2024

View reviewed changes

Add parameter for version inc workflow

14e0aac

Signed-off-by: Derek Ho <[email protected]>

AMoo-Miki approved these changes Feb 28, 2024

View reviewed changes

Add loose parameter to else block bootstrap

04a994e

Signed-off-by: Derek Ho <[email protected]>

peterzhuamazon reviewed Feb 29, 2024

View reviewed changes

This was referenced Mar 15, 2024

[AUTOCUT] Distribution Build Failed for observabilityDashboards-2.13.0 opensearch-project/dashboards-observability#1527

Closed

[AUTOCUT] Distribution Build Failed for securityDashboards-2.13.0 opensearch-project/security-dashboards-plugin#1826

Closed

peterzhuamazon mentioned this pull request Mar 15, 2024

[RELEASE] Release version 2.13.0 #4433

Closed

72 tasks

peterzhuamazon closed this Mar 26, 2024

derek-ho reopened this May 3, 2024

peterzhuamazon closed this May 3, 2024

derek-ho reopened this Jun 20, 2024

DarshitChanpura mentioned this pull request Jul 9, 2024

Fixes the span item to reflect correct text opensearch-project/opensearch-dashboards-functional-test#1439

Merged

1 task

cwperks assigned derek-ho Jul 11, 2024

Remove loose in default build

08195e2

Signed-off-by: Derek Ho <[email protected]>

peterzhuamazon approved these changes Jul 15, 2024

View reviewed changes

peterzhuamazon merged commit 8de7d53 into opensearch-project:main Jul 15, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add single version loose param for building plugins #4468

Add single version loose param for building plugins #4468

derek-ho commented Feb 21, 2024 •

edited

Loading

derek-ho commented Feb 21, 2024

derek-ho commented Feb 21, 2024

codecov bot commented Feb 21, 2024 •

edited

Loading

peterzhuamazon left a comment

ruanyl commented Feb 26, 2024

prudhvigodithi commented Feb 28, 2024

derek-ho commented Feb 28, 2024 •

edited

Loading

peterzhuamazon Feb 29, 2024

derek-ho Mar 1, 2024

gaiksaya Mar 15, 2024

prudhvigodithi commented Mar 5, 2024

derek-ho commented Mar 6, 2024

peterzhuamazon commented Mar 15, 2024

peterzhuamazon commented Mar 18, 2024 •

edited

Loading

peterzhuamazon commented Mar 26, 2024

ruanyl commented Apr 23, 2024 •

edited

Loading

derek-ho commented May 3, 2024

peterzhuamazon commented May 3, 2024

ruanyl commented May 6, 2024

derek-ho commented Jun 20, 2024 •

edited

Loading

peterzhuamazon commented Jun 20, 2024

derek-ho commented Jun 27, 2024

cwperks commented Jun 27, 2024

ruanyl commented Jun 28, 2024

derek-ho commented Jun 28, 2024

derek-ho commented Jul 12, 2024

AMoo-Miki commented Jul 12, 2024 •

edited

Loading

peterzhuamazon commented Jul 12, 2024 •

edited

Loading

peterzhuamazon commented Jul 15, 2024

Add single version loose param for building plugins #4468

Add single version loose param for building plugins #4468

Conversation

derek-ho commented Feb 21, 2024 • edited Loading

Description

Category

Why these changes are required?

What is the old behavior before changes and new behavior after changes?

Issues Resolved

Testing

Check List

derek-ho commented Feb 21, 2024

derek-ho commented Feb 21, 2024

codecov bot commented Feb 21, 2024 • edited Loading

Codecov Report

peterzhuamazon left a comment

Choose a reason for hiding this comment

ruanyl commented Feb 26, 2024

prudhvigodithi commented Feb 28, 2024

derek-ho commented Feb 28, 2024 • edited Loading

peterzhuamazon Feb 29, 2024

Choose a reason for hiding this comment

derek-ho Mar 1, 2024

Choose a reason for hiding this comment

gaiksaya Mar 15, 2024

Choose a reason for hiding this comment

prudhvigodithi commented Mar 5, 2024

derek-ho commented Mar 6, 2024

peterzhuamazon commented Mar 15, 2024

peterzhuamazon commented Mar 18, 2024 • edited Loading

peterzhuamazon commented Mar 26, 2024

ruanyl commented Apr 23, 2024 • edited Loading

derek-ho commented May 3, 2024

peterzhuamazon commented May 3, 2024

ruanyl commented May 6, 2024

derek-ho commented Jun 20, 2024 • edited Loading

peterzhuamazon commented Jun 20, 2024

derek-ho commented Jun 27, 2024

cwperks commented Jun 27, 2024

ruanyl commented Jun 28, 2024

derek-ho commented Jun 28, 2024

derek-ho commented Jul 12, 2024

AMoo-Miki commented Jul 12, 2024 • edited Loading

peterzhuamazon commented Jul 12, 2024 • edited Loading

peterzhuamazon commented Jul 15, 2024

derek-ho commented Feb 21, 2024 •

edited

Loading

codecov bot commented Feb 21, 2024 •

edited

Loading

derek-ho commented Feb 28, 2024 •

edited

Loading

peterzhuamazon commented Mar 18, 2024 •

edited

Loading

ruanyl commented Apr 23, 2024 •

edited

Loading

derek-ho commented Jun 20, 2024 •

edited

Loading

AMoo-Miki commented Jul 12, 2024 •

edited

Loading

peterzhuamazon commented Jul 12, 2024 •

edited

Loading