Onboard Maven restores to CFS

[Ahmed Muhsin (ahmedmuhsin)]

Summary

• Route Maven dependency and plugin restores through the upstream-public CFS feed.
• Override the snapshot plugin repository inherited from java-8-parent.
• Install the repository Maven mirror and authenticate it before public and official CI builds.
• Resolve the Application Insights agent through Maven instead of downloading it directly from Maven Central.
• Add local credential-provider helpers for ingesting uncached packages.

This follows the Maven onboarding pattern from Azure/azure-functions-openai-extension#218.

Validation

• Authenticated CFS mvn clean package -DskipTests -Dgpg.skip=true -Dspotbugs.skip=true
• Application Insights agent copy through Maven and CFS
• Verified the effective dependency, plugin, and mirror configuration
• Exercised the PowerShell credential-provider bootstrap using a disposable Maven repository
• Validated PowerShell and Bash helper syntax

[Swapnil Nagar (swapnil-nagar)]

Overall Assessment

The PR is well-structured and follows established patterns (mirrors azure-functions-openai-extension#218). The intent is clear, CI changes are symmetric between �uild.yml and �uild-and-test.yml, and the documentation is thorough. All 5 CI checks pass. Below are findings ranging from blockers to nits.

---

Issues

1. PowerShell here-string indentation will produce malformed-looking XML (Minor)

In Install-MavenCredentialProvider.ps1, the @"..."@ here-string picks up the code-level indentation as literal leading spaces in every XML line:

powershell $extensionsContent = @" <?xml version="1.0" encoding="UTF-8"?> ... <extensions ...> <extension>

Every line in the generated .mvn/extensions.xml will be prefixed with four spaces. The XML is still valid and Maven will parse it correctly, but it's inconsistent with the Bash script which generates clean unindented XML (via cat >"" <<EOF at column 0). Consider outdenting the here-string content to column 0 (moving the closing "@ to column 0 as well).

---

2. �uild.ps1 — mvn dependency:copy skips the mirror on CI if settings aren't copied yet (Low Risk, Worth Confirming)

The �uild.ps1 Maven invocation:
powershell & mvn @mavenArguments # uses maven-dependency-plugin:copy
doesn't pass -s settings.xml. In CI, this relies on the Install Maven settings.xml step having already run and written the mirror to ~/.m2/settings.xml. Looking at �uild.yml and �uild-and-test.yml, the copy step does run before the pwsh step that eventually calls �uild.ps1. So this is safe — but it's a subtle ordering dependency. A comment in �uild.ps1 noting the assumption would prevent future breakage.

---

3. pom.xml — central repository and settings.xml mirror are redundant but not conflicting (Nit)

The pom.xml now declares central pointing directly to the CFS URL:
xml <id>central</id> <url>https://pkgs.dev.azure.com/azfunc/public/_packaging/upstream-public/maven/v1</url>

And settings.xml also mirrors central to the same URL. When the mirror is active (CI, or local with -s settings.xml), the mirror wins. When it's not active (anonymous developer without the settings file), the pom.xml repository override still routes to CFS. Both are needed for different scenarios — the README explains this well. Just worth being aware that the same URL appears in two places and must be kept in sync if the feed URL ever changes.

---

4. pom.xml — Snapshot policy on the central CFS repo

The replaced maven.snapshots entry had false. The new central entry has both releases and snapshots enabled:
xml <releases><enabled>true</enabled></releases> <snapshots><enabled>true</enabled></snapshots>
This means Maven will query the CFS feed for snapshots too. This is intentional (you want all traffic through CFS), but any snapshot dependencies not yet cached in the feed will produce authenticated-restore errors for anonymous consumers. Confirm that all snapshot dependencies the project uses are either eliminated or already cached.

---

5. �ng/scripts/install-maven-credprovider.sh — Version regex escaping (Very Minor)

�ash if grep -qE "<version>[[:space:]]*${version//./\\.}[[:space:]]*</version>" "$extensions_path"; then

The ${version//./\.} substitution replaces . with . in the shell variable before passing to ERE. However, in ERE . is a literal dot (correct), but \ in a shell double-quoted string becomes , so the actual regex character class becomes . which is a literal dot. This is correct behavior but slightly confusing. A comment would help, or alternatively use [.] which is unambiguous in ERE. Not a bug.

---

6. settings.xml — No element (Informational)

The settings.xml uses a but does not set . This is fine for CI. Just confirming there's no intention to pin the cache location for reproducibility — there isn't, and the default ~/.m2/repository is appropriate.

---

Positives

• .gitattributes: Correctly enforces LF line endings on .sh files — essential for cross-platform correctness.
• .gitignore: Good choice to ignore .mvn/ with an explanatory comment about why committing it would break anonymous restores.
• �uild.ps1: Replacing Invoke-WebRequest directly from Maven Central with mvn dependency:copy through CFS is the right approach. Pinning maven-dependency-plugin:3.8.1 prevents version drift. StopOnFailedExecution preserves the existing error-handling pattern.
• CI templates: The ordering (install settings → MavenAuthenticate@0 → build) is correct. The comment explaining why the mirror must be installed before MavenAuthenticate@0 is valuable.
• Credential provider scripts: Both PS1 and Bash scripts are idempotent, handle the temp-dir bootstrap correctly (running outside any Maven project), and clean up reliably (inally / rap EXIT). Error messages are actionable.
• Security: No credentials committed anywhere. README clearly directs users to keep PATs only in their personal ~/.m2/settings.xml. The settings.xml at repo root is safe to commit.
• Documentation: The README additions are comprehensive and will meaningfully reduce onboarding friction.

---

Summary

Area	Finding	Severity
Install-MavenCredentialProvider.ps1	Here-string indentation produces leading whitespace in generated XML	Minor
�uild.ps1	Silent ordering dependency on CI settings copy — add comment	Low
pom.xml + settings.xml	Same CFS URL in two places, must be kept in sync	Nit
pom.xml	Snapshots enabled on CFS central — confirm all snapshots are cached	Worth confirming
install-maven-credprovider.sh	Version regex logic is correct but could use a comment	Nit

The PR is approvable. The only item worth confirming before merge is #4 (snapshot dependency coverage in the CFS feed). The PowerShell heredoc indentation (#1) is the most visible artifact and worth a quick fix.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Ahmed Muhsin (ahmedmuhsin)]

Thanks for the review. Follow-up commit 4121bc1 documents the CI ordering assumption directly in build.ps1.

A few confirmations:

• The PowerShell here-string starts at column zero and is byte-identical to the merged reference implementation; its generated XML was exercised successfully.
• The POM repository plus settings mirror are intentionally complementary, as documented in the README.
• There are no external snapshot dependency coordinates in this project. Authenticated CFS packaging and the public CI build pass.
• The Bash version regex is copied from the validated reference implementation.
• The default Maven local repository is intentional; no pinned cache path is needed.

[Swapnil Nagar (swapnil-nagar)]

Thanks for the quick follow-up — verified the new revision (4121bc1 Document Maven settings dependency) and your responses. Status of the original findings:

• build.ps1 ordering (Low) — Addressed. 4121bc1 adds the comment documenting that CI installs authenticated settings before invoking the script and local restores use the pom-declared CFS repository. ✅
• Here-string indentation (Minor) — Confirmed non-issue: the @""...""@ content in Install-MavenCredentialProvider.ps1 is at column zero on the head branch; my original note was a misread of the rendered diff. ✅
• Snapshots on central (confirm) — Confirmed: no external snapshot dependency coordinates in the project; authenticated CFS packaging and public CI pass. ✅
• POM repo + settings.xml mirror / Bash version regex / default local repository — Acknowledged as intentional and consistent with the reference implementation.

All actionable items are resolved and the remaining ones are intentional design choices with clear rationale. Approving. 🚢

[Ahmed Muhsin (ahmedmuhsin)]

Heads up, I pushed 9848e6d after your approval.

The 1ES network isolation task reports CFSClean2 as NOT COMPLIANT with 5 connections to oss.sonatype.org. That task watches live build traffic rather than file contents, so the source scan I did earlier missed it. build.ps1 clones and builds azure-maven-plugins, whose azure-functions-maven-plugin pom declares a download repository under the id maven.snapshots. Our mirror was scoped to central, so that id went straight to Sonatype.

The commit scopes the mirror to external:*. I verified locally that maven.snapshots, sonatype-nexus-snapshots and central all resolve through the feed now.

Sorry for the extra round trip. Would appreciate another look once CI finishes.