Skip to content

Create FIPS complaint boards plugin #120

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.

Sign up for GitHub

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

Open
stafot wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Create FIPS complaint boards plugin #120

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6ced12d
Create FIPS complaint boards plugin
stafot Jul 21, 2025
714a8f8
update packages
stafot Jul 21, 2025
5f7c08e
update packages 2
stafot Jul 21, 2025
68aed98
update packages 3 - set as previous main
stafot Jul 21, 2025
a11b210
update go
stafot Jul 21, 2025
c50a4d8
review and lint fixes
stafot Jul 21, 2025
86e95ad
ci fixes
stafot Jul 21, 2025
d506c13
ci fixes 2
stafot Jul 21, 2025
090d0d1
ci fixes 3
stafot Jul 21, 2025
ad995cb
ci fixes 4
stafot Jul 21, 2025
403f0f4
review fixes
stafot Aug 6, 2025
d580880
review fixes 2
stafot Aug 6, 2025
691caaa
Bump go image and apply review fixes
stafot Aug 20, 2025
e5276c1
Fixes after testing
stafot Aug 21, 2025
e12e41d
add artifacts sign
stafot Sep 3, 2025
d251b01
Remove sign from CI
stafot Sep 8, 2025
9ff99e6
Add manual artifacts CI sign for plugins - fixes
stafot Sep 8, 2025
56e2903
add version prefix
stafot Sep 11, 2025
a980cd2
Merge tag 'v9.1.7' into CLD-9417-build-boards-fips-compliant
agarciamontoro Oct 28, 2025
c150f87
Merge tag 'v9.2.1' into CLD-9417-build-boards-fips-compliant
agarciamontoro Nov 13, 2025
c9e9433
Fix merge
agarciamontoro Nov 13, 2025
df49b26
Make the linter happy
agarciamontoro Nov 13, 2025
4282c63
Merge tag 'v9.2.2' into CLD-9417-build-boards-fips-compliant
agarciamontoro Jan 26, 2026
457dfa5
Merge tag 'v9.2.4' into CLD-9417-build-boards-fips-compliant
agarciamontoro Apr 17, 2026
5855fe1
Trigger dist
agarciamontoro Apr 17, 2026
f4fc5d6
Merge tag 'v9.2.5' into CLD-9417-build-boards-fips-compliant
avasconcelos114 May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 112 additions & 7 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ on:
env:
BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
EXCLUDE_ENTERPRISE: true
GO_VERSION: 1.24.6

permissions:
contents: read
Expand Down Expand Up @@ -41,14 +42,118 @@ jobs:
run: cd focalboard; make webapp-ci

- name: set up golangci-lint
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.64.8
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.1.6

- name: Lint & test server
run: cd focalboard; make server-ci

dist:
uses: mattermost/actions-workflows/.github/workflows/plugin-dist-pr.yml@main
secrets: inherit
with:
dist-target: "dist-linux"
s3-prefix: "mattermost-plugin-boards"
runs-on: ubuntu-22.04
needs:
- webapp-test
permissions:
id-token: write
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
path: "focalboard"
fetch-depth: "0"

- name: Set up Go
uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0
with:
go-version: "${{ env.GO_VERSION }}"
cache: true

- name: Setup Node
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0
with:
node-version-file: focalboard/.nvmrc
cache: "npm"
cache-dependency-path: focalboard/webapp/package-lock.json

- name: Cache webapp node modules
id: cache-webapp-node-modules
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
with:
path: focalboard/webapp/node_modules
key: ${{ runner.os }}-webapp-node-modules-${{ hashFiles('focalboard/webapp/package-lock.json') }}
restore-keys: ${{ runner.os }}-webapp-node-modules-

- name: Setup webapp npm deps
if: steps.cache-webapp-node-modules.outputs.cache-hit != 'true'
env:
NODE_ENV: development
run: |
cd focalboard/webapp
npm install --ignore-scripts --no-save

- name: ci/setup-chainctl
uses: chainguard-dev/setup-chainctl@v0.3.2
with:
identity: ${{ secrets.CHAINGUARD_IDENTITY }}

- name: ci/setup-build-tools
run: |
echo "Setting up build tools..."
cd focalboard
mkdir -p build/bin
cd build/manifest && go build -o ../bin/manifest
cd ../pluginctl && go build -o ../bin/pluginctl

- name: Build both distributions
env:
GO_VERSION: ${{ env.GO_VERSION }}
run: cd focalboard; make dist-all
Comment on lines +96 to +128

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Webapp cache + install steps are nullified by make dist-all’s clean target.

dist-all declares clean as a prerequisite (see Makefile Line 285), and clean does rm -fr webapp/node_modules (Makefile Line 434). So the Cache webapp node modules and Setup webapp npm deps steps here (Lines 76–90) are always thrown away before the actual build — dist → webapp → webapp/node_modules triggers a fresh npm install anyway.

Either drop these two steps, or remove clean from dist-all’s prerequisites (preferable — see my comment on the Makefile) so the cache actually buys you something.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 76 - 108, The CI currently caches and
installs focalboard/webapp node_modules in the steps named "Cache webapp node
modules" and "Setup webapp npm deps", but those are always deleted because the
Makefile target dist-all declares clean as a prerequisite (target dist-all ->
clean) and clean removes webapp/node_modules; fix by removing clean from the
dist-all prerequisites in the Makefile (edit the dist-all target to no longer
depend on clean) so the cached node_modules survive the make dist-all run, or
alternatively remove the two CI steps ("Cache webapp node modules" and "Setup
webapp npm deps") from .github/workflows/ci.yml if you prefer to keep clean in
dist-all.


- name: ci/display-signing-parameters
if: github.event_name == 'pull_request'
run: |
cd focalboard
echo "📦 Plugin Artifact Signing Parameters"
echo "===================================="

# Extract plugin version using manifest tool (handles git versioning)
PLUGIN_VERSION=$(build/bin/manifest version)
# Add 'v' prefix for consistency with other plugin packages
if [[ ! $PLUGIN_VERSION == v* ]]; then
PLUGIN_VERSION="v${PLUGIN_VERSION}"
fi
SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7)

echo ""
echo "To sign artifacts from this PR, run the following command:"
echo ""
echo "gh workflow run sign-plugin-pr-artifacts.yaml \\"
echo " --repo mattermost/delivery-platform \\"
echo " --field repository_full_name=\"${{ github.repository }}\" \\"
echo " --field pr_number=\"${{ github.event.number }}\" \\"
echo " --field commit_sha=\"${{ github.sha }}\" \\"
echo " --field run_id=\"${{ github.run_id }}\" \\"
echo " --field plugin_version=\"${PLUGIN_VERSION}\" \\"
echo " --field include_fips=true"
echo ""
echo "Or use the GitHub web interface with these values:"
echo "- Repository Full Name: ${{ github.repository }}"
echo "- PR Number: ${{ github.event.number }}"
echo "- Commit SHA: ${{ github.sha }}"
echo "- Run ID: ${{ github.run_id }}"
echo "- Plugin Version: ${PLUGIN_VERSION}"
echo "- Include FIPS: true"
echo ""
echo "Expected artifact naming:"
echo "- mattermost-plugin-boards-${PLUGIN_VERSION}+${SHORT_SHA}-linux-amd64.tar.gz"
echo "- mattermost-plugin-boards-${PLUGIN_VERSION}+${SHORT_SHA}-fips-linux-amd64.tar.gz"
echo ""
echo "Artifacts will be available at:"
echo "https://plugins.releases.mattermost.com/pr/mattermost-plugin-boards/pr-${{ github.event.number }}-${SHORT_SHA}/"

- name: Upload all artifacts
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.5.0
with:
name: all-plugin-artifacts
path: |
focalboard/dist/*.tar.gz
focalboard/dist-fips/*.tar.gz
retention-days: 7
2 changes: 1 addition & 1 deletion .github/workflows/lint-server.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
with:
go-version-file: focalboard/go.mod
- name: set up golangci-lint
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.64.8
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.1.6
- name: lint
run: |
cd focalboard
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ webapp/node_modules
webapp/dist
webapp/pack
dist
dist-fips
.build-cache/
package
bin
debug
Expand Down
175 changes: 141 additions & 34 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: prebuild clean cleanall ci server server-linux server-mac server-win server-linux-package generate watch-server webapp mac-app win-app-wpf linux-app modd-precheck templates-archive
.PHONY: prebuild clean cleanall ci server server-mac server-linux server-win server-linux-package generate watch-server webapp mac-app win-app-wpf linux-app modd-precheck templates-archive dist-all

PACKAGE_FOLDER = focalboard

Expand Down Expand Up @@ -38,7 +38,14 @@ MATTERMOST_PLUGINS_PATH=$(MM_SERVER_PATH)/plugins
BOARD_PLUGIN_PATH=$(MATTERMOST_PLUGINS_PATH)/boards
PLUGIN_NAME=boards

export GO111MODULE=on
# FIPS Support - similar to mattermost server
# To build FIPS-compliant plugin: make dist-fips
# Requires Docker to be installed and running
FIPS_IMAGE ?= cgr.dev/mattermost.com/go-msft-fips:1.24.6@sha256:b94d424ab26b590163634001b22242ceac6f5d76bfbbaa77b6f0dda97220c717

# We need to export GOBIN to allow it to be set
# for processes spawned from the Makefile
export GOBIN ?= $(PWD)/bin

ASSETS_DIR ?= assets

Expand All @@ -51,7 +58,15 @@ default: all
# Verify environment, and define PLUGIN_ID, PLUGIN_VERSION, HAS_SERVER and HAS_WEBAPP as needed.
include build/setup.mk

BUNDLE_NAME ?= $(PLUGIN_NAME)-$(PLUGIN_VERSION).tar.gz
BUNDLE_NAME ?= $(PLUGIN_ID)-$(PLUGIN_VERSION).tar.gz
Comment thread
stafot marked this conversation as resolved.

# Helper function to copy common bundle files
define copy_bundle_files
$(if $(wildcard LICENSE.txt),cp -r LICENSE.txt $(1)/$(PLUGIN_ID)/)
$(if $(wildcard NOTICE.txt),cp -r NOTICE.txt $(1)/$(PLUGIN_ID)/)
$(if $(wildcard $(ASSETS_DIR)/.),cp -r $(ASSETS_DIR) $(1)/$(PLUGIN_ID)/)
$(if $(HAS_PUBLIC),cp -r public $(1)/$(PLUGIN_ID)/public/)
endef

# Include custom makefile, if present
ifneq ($(wildcard build/custom.mk),)
Expand Down Expand Up @@ -114,6 +129,48 @@ else
endif
endif

## Builds the server with FIPS compliance using Docker (requires Docker)
.PHONY: server-fips
server-fips: templates-archive
ifneq ($(HAS_SERVER),)
@echo Building FIPS-compliant plugin server binaries
mkdir -p server/dist-fips
@echo "Setting up FIPS build environment..."

# Docker authentication is handled by CI (setup-chainctl)
@if ! docker manifest inspect $(FIPS_IMAGE) >/dev/null 2>&1; then \
echo "Docker authentication failed. Ensure setup-chainctl configured Docker authentication."; \
echo "Trying fallback authentication if credentials are available..."; \
if [ -n "$(CHAINGUARD_DEV_USERNAME)" ] && [ -n "$(CHAINGUARD_DEV_TOKEN)" ]; then \
echo "Using username/token authentication..."; \
echo "$(CHAINGUARD_DEV_TOKEN)" | docker login cgr.dev --username "$(CHAINGUARD_DEV_USERNAME)" --password-stdin; \
else \
echo "Warning: No authentication available. FIPS build may fail."; \
fi; \
else \
echo "✅ Docker authentication is working"; \
fi
Comment on lines +140 to +152

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fallback Docker login expands as Make variables, not shell env.

$(CHAINGUARD_DEV_USERNAME) and $(CHAINGUARD_DEV_TOKEN) are Make variables and will be expanded at parse time — if they aren't passed on the make command line they evaluate to empty strings, so both [ -n "" ] tests always fail silently and the "Warning: No authentication available" branch is taken regardless of what's in the CI environment. To read the CI-provided env vars, escape with $$:

🛡️ Suggested fix 
-		if [ -n "$(CHAINGUARD_DEV_USERNAME)" ] && [ -n "$(CHAINGUARD_DEV_TOKEN)" ]; then \
+		if [ -n "$$CHAINGUARD_DEV_USERNAME" ] && [ -n "$$CHAINGUARD_DEV_TOKEN" ]; then \
 			echo "Using username/token authentication..."; \
-			echo "$(CHAINGUARD_DEV_TOKEN)" | docker login cgr.dev --username "$(CHAINGUARD_DEV_USERNAME)" --password-stdin; \
+			echo "$$CHAINGUARD_DEV_TOKEN" | docker login cgr.dev --username "$$CHAINGUARD_DEV_USERNAME" --password-stdin; \
 		else \

Since CI is expected to use setup-chainctl for auth, this is low severity — but as-written the fallback is dead code.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Docker authentication is handled by CI (setup-chainctl)
@if ! docker manifest inspect $(FIPS_IMAGE) >/dev/null 2>&1; then \
echo "Docker authentication failed. Ensure setup-chainctl configured Docker authentication."; \
echo "Trying fallback authentication if credentials are available..."; \
if [ -n "$(CHAINGUARD_DEV_USERNAME)" ] && [ -n "$(CHAINGUARD_DEV_TOKEN)" ]; then \
echo "Using username/token authentication..."; \
echo "$(CHAINGUARD_DEV_TOKEN)" | docker login cgr.dev --username "$(CHAINGUARD_DEV_USERNAME)" --password-stdin; \
else \
echo "Warning: No authentication available. FIPS build may fail."; \
fi; \
else \
echo "✅ Docker authentication is working"; \
fi
# Docker authentication is handled by CI (setup-chainctl)
`@if` ! docker manifest inspect $(FIPS_IMAGE) >/dev/null 2>&1; then \
echo "Docker authentication failed. Ensure setup-chainctl configured Docker authentication."; \
echo "Trying fallback authentication if credentials are available..."; \
if [ -n "$$CHAINGUARD_DEV_USERNAME" ] && [ -n "$$CHAINGUARD_DEV_TOKEN" ]; then \
echo "Using username/token authentication..."; \
echo "$$CHAINGUARD_DEV_TOKEN" | docker login cgr.dev --username "$$CHAINGUARD_DEV_USERNAME" --password-stdin; \
else \
echo "Warning: No authentication available. FIPS build may fail."; \
fi; \
else \
echo "✅ Docker authentication is working"; \
fi
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 140 - 152, The fallback Docker-login branch in the
Makefile is using Makefile expansion
($(CHAINGUARD_DEV_USERNAME)/$(CHAINGUARD_DEV_TOKEN)) instead of reading the CI
shell environment, so the tests [ -n "$(CHAINGUARD_DEV_USERNAME)" ] always fail;
change those Make-variable references to shell env references by escaping the
dollar sign (use $$CHAINGUARD_DEV_USERNAME and $$CHAINGUARD_DEV_TOKEN wherever
they appear in the conditional and the docker login command) so the shell
invoked by the recipe reads the CI-provided environment variables at runtime.


# Create local cache directory for CI/ACT compatibility
mkdir -p $(PWD)/.build-cache

# Try FIPS build with error handling
@if docker run --rm \
--entrypoint="" \
-v $(PWD):/plugin \
-v $(PWD)/.build-cache:/root/.cache \
-w /plugin/server \
$(FIPS_IMAGE) \
sh -c "CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go build -tags fips -ldflags '$(LDFLAGS)' -trimpath -buildvcs=false -o dist-fips/plugin-linux-amd64-fips"; then \
echo "FIPS plugin server build completed: server/dist-fips/plugin-linux-amd64-fips"; \
else \
echo "FIPS build failed - likely authentication issue with $(FIPS_IMAGE)"; \
echo "Creating placeholder to indicate FIPS build was attempted but failed"; \
echo "FIPS_BUILD_FAILED" > server/dist-fips/FIPS_BUILD_FAILED.txt; \
exit 1; \
fi
endif
Comment on lines +132 to +172

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Does cgr.dev/mattermost.com/go-msft-fips:1.24 support cross-compilation to linux/arm64 with CGO enabled, or is it amd64-only?

💡 Result:

No, cgr.dev/mattermost.com/go-msft-fips:1.24 does not support cross-compilation to linux/arm64 with CGO enabled. It is amd64-only. The image is used as a builder in Mattermost FIPS Dockerfiles with CGO_ENABLED=1 explicitly set for FIPS compliance via Microsoft's Go build with systemcrypto and OpenSSL CGO bindings. Mattermost's FIPS builds explicitly limit to linux/amd64 ("only build-linux-amd64 for fips", "skip arm for FIPS"), skipping arm64. Chainguard FIPS images like mattermost-fips list sizes only for x86_64 (amd64), with no arm64 variants. FIPS Go requires CGO for OpenSSL, and cross-compilation with CGO to arm64 needs cross-compiled OpenSSL libraries, which this specialized Chainguard image likely lacks—consistent with amd64-only builds in practice.

Citations:

FIPS build scope is intentionally limited to linux-amd64 by the FIPS tooling; document this constraint and improve error messaging.

The server-fips target only produces plugin-linux-amd64-fips because the underlying FIPS Docker image (cgr.dev/mattermost.com/go-msft-fips:1.24) is amd64-only. Cross-compilation to arm64 is not supported by this image—FIPS Go requires CGO bindings for OpenSSL/systemcrypto, and the image lacks cross-compiled libraries for non-x86_64 platforms. This is a known architectural decision in Mattermost FIPS builds.

To prevent silent failures when admins install the FIPS bundle on non-amd64 hosts:

  1. Document the amd64-only scope in a comment above this target or in the PR description.
  2. Ensure the server surfaces a clear error if the host architecture doesn't match the available FIPS binary.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 132 - 172, The server-fips target produces only an
amd64 binary (server/dist-fips/plugin-linux-amd64-fips) because the FIPS Docker
image (FIPS_IMAGE) is amd64-only; add a short comment above the server-fips
target documenting this amd64-only constraint and why cross-compilation isn't
supported. Also add an explicit host-architecture check in the server-fips
recipe (use uname -m or similar) and abort early with a clear error message if
the host is not x86_64, writing the FIPS_BUILD_FAILED.txt placeholder and
exiting non-zero so admins installing the bundle see a deterministic failure;
reference the server-fips target, FIPS_IMAGE, output filename, and
FIPS_BUILD_FAILED.txt when making the changes.


## Builds the server, if it exists, for Linux architectures only.
.PHONY: server-linux
server-linux: templates-archive
Expand Down Expand Up @@ -150,33 +207,62 @@ endif
## Generates a tar bundle of the plugin for install.
.PHONY: bundle
bundle:
rm -rf dist/
mkdir -p dist/$(PLUGIN_NAME)
cp $(MANIFEST_FILE) dist/$(PLUGIN_NAME)/
cp -r webapp/pack dist/$(PLUGIN_NAME)/
ifneq ($(wildcard LICENSE.txt),)
cp -r LICENSE.txt dist/$(PLUGIN_NAME)/
endif
ifneq ($(wildcard NOTICE.txt),)
cp -r NOTICE.txt dist/$(PLUGIN_NAME)/
endif
ifneq ($(wildcard $(ASSETS_DIR)/.),)
cp -r $(ASSETS_DIR) dist/$(PLUGIN_NAME)/
rm -rf dist/$(PLUGIN_ID)
mkdir -p dist/$(PLUGIN_ID)
Comment thread
stafot marked this conversation as resolved.
cp $(MANIFEST_FILE) dist/$(PLUGIN_ID)/
cp -r webapp/pack dist/$(PLUGIN_ID)/
$(call copy_bundle_files,dist)
ifneq ($(HAS_SERVER),)
mkdir -p dist/$(PLUGIN_ID)/server
cp -r server/dist dist/$(PLUGIN_ID)/server/
endif
ifneq ($(HAS_PUBLIC),)
cp -r public dist/$(PLUGIN_NAME)/public/
ifneq ($(HAS_WEBAPP),)
mkdir -p dist/$(PLUGIN_ID)/webapp
cp -r webapp/dist dist/$(PLUGIN_ID)/webapp/
endif
cd dist && tar -cvzf $(BUNDLE_NAME) $(PLUGIN_ID)

@echo "==> Normal plugin built at: dist/$(BUNDLE_NAME)"

## Generates a tar bundle of the FIPS plugin for install.
.PHONY: bundle-fips
bundle-fips:
rm -rf dist-fips/
mkdir -p dist-fips/$(PLUGIN_ID)
./build/bin/manifest dist-fips
$(call copy_bundle_files,dist-fips)
ifneq ($(HAS_SERVER),)
mkdir -p dist/$(PLUGIN_NAME)/server
cp -r server/dist dist/$(PLUGIN_NAME)/server/
mkdir -p dist-fips/$(PLUGIN_ID)/server/dist
# Copy FIPS binaries but rename them to standard names for server compatibility
if [ -f server/dist-fips/plugin-linux-amd64-fips ]; then \
cp server/dist-fips/plugin-linux-amd64-fips dist-fips/$(PLUGIN_ID)/server/dist/plugin-linux-amd64; \
fi
# Copy any other FIPS binaries and rename them
for file in server/dist-fips/plugin-*-fips*; do \
if [ -f "$$file" ]; then \
target=$$(basename "$$file" | sed 's/-fips//g'); \
cp "$$file" "dist-fips/$(PLUGIN_ID)/server/dist/$$target"; \
fi; \
done
endif
ifneq ($(HAS_WEBAPP),)
mkdir -p dist/$(PLUGIN_NAME)/webapp
cp -r webapp/dist dist/$(PLUGIN_NAME)/webapp/
if [ -d webapp/dist ]; then \
mkdir -p dist-fips/$(PLUGIN_ID)/webapp && \
cp -r webapp/dist dist-fips/$(PLUGIN_ID)/webapp/; \
else \
echo "Error: webapp/dist not found, but HAS_WEBAPP is set. Run 'make webapp' first."; \
exit 1; \
fi
endif
# Use webpack pack for webapp bundle
cp -r webapp/pack dist-fips/$(PLUGIN_ID)/
ifeq ($(shell uname),Darwin)
cd dist-fips && tar --disable-copyfile -cvzf $(PLUGIN_ID)-$(PLUGIN_VERSION)-fips.tar.gz $(PLUGIN_ID)
else
cd dist-fips && tar -cvzf $(PLUGIN_ID)-$(PLUGIN_VERSION)-fips.tar.gz $(PLUGIN_ID)
endif
cd dist && tar -cvzf $(BUNDLE_NAME) $(PLUGIN_NAME)

@echo plugin built at: dist/$(BUNDLE_NAME)
@echo "==> FIPS plugin built at: dist-fips/$(PLUGIN_ID)-$(PLUGIN_VERSION)-fips.tar.gz"

info: ## Display build information
@echo "Build Number: $(BUILD_NUMBER)"
Expand All @@ -190,6 +276,25 @@ info: ## Display build information
.PHONY: dist
dist: apply server webapp bundle

## Builds and bundles the FIPS plugin.
.PHONY: dist-fips
dist-fips: apply server-fips webapp bundle-fips

## Builds both normal and FIPS distributions.
.PHONY: dist-all
dist-all: clean
@echo "==> Building both normal and FIPS distributions in parallel..."
$(MAKE) dist
@if $(MAKE) dist-fips; then \
echo "==> Both distributions built successfully:"; \
echo " Normal: dist/$$(./build/bin/manifest id)-$$(./build/bin/manifest version).tar.gz"; \
echo " FIPS: dist-fips/$$(./build/bin/manifest id)-$$(./build/bin/manifest version)-fips.tar.gz"; \
else \
echo "==> FIPS build failed, continuing with normal distribution only:"; \
echo " Normal: dist/$$(./build/bin/manifest id)-$$(./build/bin/manifest version).tar.gz"; \
echo " FIPS: Build failed - check Docker/credentials"; \
fi
Comment on lines +283 to +296

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

dist-all builds the webapp twice and mislabels work as parallel.

Two issues in this recipe:

  1. Sequential, not parallel. The comment says "Building both normal and FIPS distributions in parallel…" but $(MAKE) dist and $(MAKE) dist-fips are invoked back-to-back. Update the message, or run them with -j if parallel is really desired (harder — both targets mutate webapp/pack and server/dist).
  2. Duplicate webapp build. dist depends on webapp, and dist-fips also depends on webapp (Line 281). The webapp target has no staleness check, so npm run build + npm run pack run twice on every dist-all. Since dist-all: clean has already wiped webapp/node_modules, you also pay one npm install cost. On CI this roughly doubles the webapp portion of the build.

Recommended: drop webapp from dist-fips's prerequisites and rely on dist-all ordering, or refactor the webapp target to be idempotent. And update the echo:

♻️ Suggested 
-## Builds and bundles the FIPS plugin.
-.PHONY: dist-fips
-dist-fips: apply server-fips webapp bundle-fips
+## Builds and bundles the FIPS plugin. Assumes `webapp` has already been built
+## (e.g. when invoked via `dist-all`). Run `make dist` first if invoking standalone.
+.PHONY: dist-fips
+dist-fips: apply server-fips bundle-fips
-	`@echo` "==> Building both normal and FIPS distributions in parallel..."
+	`@echo` "==> Building normal and FIPS distributions sequentially..."
 	$(MAKE) dist
 	`@if` $(MAKE) dist-fips; then \

Also note: because dist-all: clean removes webapp/node_modules, the Cache webapp node modules / Setup webapp npm deps steps in ci.yml are effectively no-ops — see my comment on ci.yml Lines 76–108.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 283 - 296, The dist-all target currently claims a
parallel build but invokes $(MAKE) dist and $(MAKE) dist-fips sequentially and
causes the webapp to be built twice because both dist and dist-fips depend on
the webapp target; fix by either (A) adjusting the echo to remove the "parallel"
claim and keep sequential invocation, or (B) avoid duplicate webapp builds by
removing webapp from the prerequisites of the dist-fips target (so dist-all runs
dist then dist-fips reusing the already-built webapp) or by making the webapp
target idempotent; update the dist-all message string accordingly and ensure
references to webapp/pack and server/dist remain correct when you remove the
duplicate prerequisite.


## Builds and bundles the plugin for Linux only.
.PHONY: dist-linux
dist-linux: apply server-linux webapp bundle
Expand Down Expand Up @@ -315,11 +420,13 @@ kill: detach
.PHONY: clean
clean:
rm -rf bin
rm -rf dist
rm -rf dist/
rm -rf dist-fips/
rm -rf webapp/pack
ifneq ($(HAS_SERVER),)
rm -fr server/coverage.txt
rm -fr server/dist
rm -fr server/dist-fips
endif
ifneq ($(HAS_WEBAPP),)
rm -fr webapp/junit.xml
Expand Down Expand Up @@ -355,17 +462,17 @@ live-watch-webapp: apply
.PHONY: deploy-to-mattermost-directory
deploy-to-mattermost-directory:
./build/bin/pluginctl disable $(PLUGIN_ID)
mkdir -p $(FOCALBOARD_PLUGIN_PATH)
cp $(MANIFEST_FILE) $(FOCALBOARD_PLUGIN_PATH)/
cp -r webapp/pack $(FOCALBOARD_PLUGIN_PATH)/
cp -r $(ASSETS_DIR) $(FOCALBOARD_PLUGIN_PATH)/
cp -r public $(FOCALBOARD_PLUGIN_PATH)/
mkdir -p $(FOCALBOARD_PLUGIN_PATH)/server
cp -r server/dist $(FOCALBOARD_PLUGIN_PATH)/server/
mkdir -p $(FOCALBOARD_PLUGIN_PATH)/webapp
cp -r webapp/dist $(FOCALBOARD_PLUGIN_PATH)/webapp/
mkdir -p $(BOARD_PLUGIN_PATH)
cp $(MANIFEST_FILE) $(BOARD_PLUGIN_PATH)/
cp -r webapp/pack $(BOARD_PLUGIN_PATH)/
cp -r $(ASSETS_DIR) $(BOARD_PLUGIN_PATH)/
cp -r public $(BOARD_PLUGIN_PATH)/
mkdir -p $(BOARD_PLUGIN_PATH)/server
cp -r server/dist $(BOARD_PLUGIN_PATH)/server/
mkdir -p $(BOARD_PLUGIN_PATH)/webapp
cp -r webapp/dist $(BOARD_PLUGIN_PATH)/webapp/
./build/bin/pluginctl enable $(PLUGIN_ID)
@echo plugin built at: $(FOCALBOARD_PLUGIN_PATH)
@echo plugin built at: $(BOARD_PLUGIN_PATH)

# Help documentation à la https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html
help:
Expand Down
Loading
Loading