feat: plugin location support for GOARCH and GOOS env vars

[mgs255]

Signed-off-by: Michael Sommerville [email protected]

Closes: #3279

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

[sonarcloud]

Quality Gate failed

Failed conditions

48.6% Duplication on New Code (required ≤ 40%)

See analysis details on SonarCloud

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.89%. Comparing base (8405f2e) to head (26efbce).
Report is 77 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3280      +/-   ##
==========================================
+ Coverage   81.83%   82.89%   +1.05%     
==========================================
  Files         135      135              
  Lines       20688    17149    -3539     
==========================================
- Hits        16931    14215    -2716     
+ Misses       2883     2042     -841     
- Partials      874      892      +18     

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

[github-actions]

Go Published Test Results

2 161 tests   2 161 ✅  2m 53s ⏱️
  119 suites      0 💤
    1 files        0 ❌

Results for commit 26efbce.

♻️ This comment has been updated with latest results.

[github-actions]

E2E Tests Published Test Results

  6 files    6 suites   5h 23m 33s ⏱️
110 tests  98 ✅  6 💤  6 ❌
694 runs  624 ✅ 36 💤 34 ❌

For more details on these failures, see this check.

Results for commit 26efbce.

♻️ This comment has been updated with latest results.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
5.4% Duplication on New Code

See analysis details on SonarCloud

[zachaller]

Would be awesome to document this:
https://argo-rollouts.readthedocs.io/en/stable/analysis/plugins/#using-a-https-server-to-host-the-plugin-executable

https://argo-rollouts.readthedocs.io/en/stable/features/traffic-management/plugins/#using-a-https-server-to-host-the-plugin-executable

[mgs255]

@zachaller Thanks for reviewing and sorry for the delay responding.

I started trying to document this and I realised that the approach I've taken really is not compatible with the checksum field. If considered important it might have been better to support an extended plugin syntax, perhaps something like:

  - name: plugin-name
    platforms:
      linux-amd64: 
        location: <uri>
        sha256: <checksum>
      linux-arm64: 
        location: <uri>
        sha256: <checksum>

Presumably, for backwards compatibility, this would need to continue supporting the existing syntax complicating the implementation a bit, e.g:

apiVersion: v1
kind: ConfigMap
metadata:
  name: argo-rollouts-config
data:
  trafficRouterPlugins: |-
    - name: "argoproj-labs/sample-nginx" # name of the plugin, it must match the name required by the plugin so it can find it's configuration
      location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-nginx/releases/download/v0.0.1/metric-plugin-linux-amd64" # supports http(s):// urls and file://
      sha256: "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2c" #optional sha256 checksum of the plugin executable
    - name: "argoproj-labs/sample-httproute"
      platform:  
        linux-amd64: 
          location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-httproute/releases/download/v0.0.1/httproute-plugin-linux-amd64"
          sha256: "<checksum>"
        linux-arm64: 
          location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-httproute/releases/download/v0.0.1/httproute-plugin-linux-arm64"
          sha256: "<checksum>"

Personally, I'd always self-host these binaries anyway rather than downloading from the internet, so omitting the checksum wouldn't be such a concern. What do you think?

[zachaller]

Yea that is a good callout another option would be to allow a list of sha's that are valid, it's a little bit less of a spec change but also maybe not as direct from a security perspective not sure yet thoughts?

trafficRouterPlugins: |-
    - name: "argoproj-labs/sample-nginx" # name of the plugin, it must match the name required by the plugin so it can find it's configuration
      location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-nginx/releases/download/v0.0.1/metric-plugin-linux-amd64" # supports http(s):// urls and file://
      sha256s: 
        - "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2c"
        - "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2d"

or with some extra metadata

trafficRouterPlugins: |-
    - name: "argoproj-labs/sample-nginx" # name of the plugin, it must match the name required by the plugin so it can find it's configuration
      location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-nginx/releases/download/v0.0.1/metric-plugin-linux-amd64" # supports http(s):// urls and file://
      sha256s: 
        -  name: linux-arm64
           sha: "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2c"
        -  name: linux-amd64
           sha: "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2d"

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
12.1% Duplication on New Code

See analysis details on SonarCloud