[MM-69594] Write resolved manifest (with version) into bundled plugin - #232
Conversation
….json (#217) * [MM-69594] Write resolved manifest (with version) into bundled plugin.json The bundle Makefile target copied the source plugin.json verbatim, which has no version field, so released boards bundles shipped a plugin.json without a version. Add a `dist` subcommand to the manifest tool that writes the resolved manifest (with the build-time version and release notes URL) into the bundle directory, and use it in the bundle target, matching the playbooks / starter-template pattern. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69594] Add tests for manifest dist subcommand Refactor distManifest to accept the destination directory as a parameter for testability, and add tests covering manifest distribution and the build-time version fallback in findManifest. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69594] Expand manifest tests for version and release-notes resolution Cover the latest-tag fallback, release-notes URL generation, the version-already-present path, and an end-to-end regression test that the repo source plugin.json (which ships without a version) yields a versioned plugin.json in the bundle. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Document sequential test ordering for writeTempManifest helpers Add comments noting that tests using writeTempManifest or mutating process-wide cwd/build vars must not call t.Parallel(). Co-authored-by: mattermost-code <matty-code@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> # Conflicts: # Makefile
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 153-156: Update the bundle target to use $(BUNDLE_DIR)
consistently for all staging, archive, directory-change, and output-message
paths. Replace every remaining dist reference, including the cd dist command and
final echo, while preserving the existing bundle generation flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3aae17c8-c977-4f25-80f1-7a1a489964d9
📒 Files selected for processing (3)
Makefilebuild/manifest/main.gobuild/manifest/main_test.go
* Add FIPS build support for Linux/amd64 plugin releases. Introduces build/fips.mk and CI dist-fips job so delivery-platform can produce FIPS-compliant artifacts alongside standard builds. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix FIPS docker build ldflags quoting. Boards embeds single-quoted -ldflags in GO_BUILD_FLAGS, which broke the docker inner shell and caused go build to omit -o. Co-authored-by: Cursor <cursoragent@cursor.com> * Gate FIPS CI on test jobs and disambiguate artifact names. Align dist-fips with the Playbooks FIPS workflow so builds run only after server and webapp CI pass, and artifact uploads include the short SHA. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # Makefile
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
159-162: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
bundleneeds an explicit manifest prerequisite
build/bin/manifestis generated, andbundleinvokes it directly. Add the existing manifest-build target as a normal prerequisite somake clean && make bundledoesn’t depend on an already-built binary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 159 - 162, Update the Makefile’s bundle target to declare the existing manifest-build target as an explicit prerequisite before its commands, ensuring build/bin/manifest is generated during make clean && make bundle while preserving the current bundling steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 105-107: Update the “Get short SHA” step in the short-sha workflow
to use github.event.pull_request.head.sha when available and fall back to
github.sha for push events, then continue truncating the selected SHA to seven
characters before writing the sha output.
In `@build/fips.mk`:
- Around line 56-67: Update the FIPS target dependency graph so verify-fips
explicitly depends on server-fips, and bundle-fips explicitly depends on all
required staging inputs, including webapp and manifest preparation, rather than
relying on prerequisite ordering. Preserve the existing build commands and
ensure parallel make execution cannot verify or package before those inputs
complete.
---
Outside diff comments:
In `@Makefile`:
- Around line 159-162: Update the Makefile’s bundle target to declare the
existing manifest-build target as an explicit prerequisite before its commands,
ensuring build/bin/manifest is generated during make clean && make bundle while
preserving the current bundling steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ba3b7c07-cc6a-40cd-b226-ad1ed639c97d
📒 Files selected for processing (3)
.github/workflows/ci.ymlMakefilebuild/fips.mk
Summary
This PR cherry picks the version fix for the plugin manifest onto the 9.2 release branch
Change Impact: Medium 🟡
Regression Risk: Build and manifest changes span multiple release components; automated tests cover manifest resolution, while FIPS packaging and CI paths carry limited regression risk.
QA Recommendation: Manual QA can be skipped; rely on automated tests and CI validation of standard and FIPS builds.
Generated by CodeRabbitAI