Skip to content

BUILD-11500 Use self-hosted Artifactory for Maven actions - #280

Closed
hedinasr wants to merge 16 commits into
masterfrom
feat/hnasr/BUILD-11500-configMavenArtifactoryInputs
Closed

BUILD-11500 Use self-hosted Artifactory for Maven actions#280
hedinasr wants to merge 16 commits into
masterfrom
feat/hnasr/BUILD-11500-configMavenArtifactoryInputs

Conversation

@hedinasr

@hedinasr hedinasr commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

BUILD-11500 This PR switches config-maven, build-maven (via config-maven), and promote to the self-hosted JFrog instance at https://repox.dev.sonar.build/artifactory.

Changes

  • Remove repox-url and repox-artifactory-url inputs from config-maven, build-maven, and promote.
  • Set ARTIFACTORY_URL to https://repox.dev.sonar.build/artifactory in the composite action steps (not configurable).
  • Keep Vault for Artifactory credentials (private-reader / public-reader, deployer, promoter) and document the fixed URL in the README.

Breaking changes

  • Workflows that pass repox-url or repox-artifactory-url to config-maven, build-maven, or promote must drop those inputs.

Other actions (config-gradle, config-pip, build-npm, etc.) still use repox-* URL inputs and are unchanged in this PR.

Prerequisites (Platform)

  • Vault development/artifactory mount must reach repox.dev.sonar.build and issue tokens valid for that instance.
  • Repository Vault roles (*-reader, *-qa-deployer, *-promoter, etc.) must exist for the new instance as needed.

Test plan

  • config-maven on a sample repo resolves dependencies from repox.dev.sonar.build.
  • build-maven build and deploy still work end-to-end.
  • promote promotes a build against the same instance.

Summary by Gitar

  • Action configuration:
    • Removed repox-url and repox-artifactory-url inputs from config-maven, build-maven, and promote actions.
    • Hardcoded ARTIFACTORY_URL to https://repox-internal.dev.sonar.build/artifactory across core Maven actions.
  • Authentication and Security:
    • Standardized vault-action-wrapper calls to use https://vault.sonar.build exclusively.
    • Streamlined ARTIFACTORY_DEPLOY secret mapping to extract credentials directly from Vault outputs.
  • Maven environment:
    • Updated updatePolicy to always in settings.xml to ensure fresh metadata resolution.
  • Documentation updates:
    • Updated README.md to clarify self-hosted Artifactory usage and reflect the removal of repox-url inputs.
    • Corrected contents permission requirement from write to read for workflows.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jun 1, 2026

Copy link
Copy Markdown

BUILD-11500

Comment thread build-maven/action.yml Outdated
Comment thread config-maven/action.yml Outdated
Comment thread build-maven/action.yml Outdated
Comment thread README.md Outdated
@hedinasr
hedinasr force-pushed the feat/hnasr/BUILD-11500-configMavenArtifactoryInputs branch from 232cc55 to b4665bb Compare June 2, 2026 09:23
@hedinasr hedinasr changed the title BUILD-11500 Add Artifactory URL and credential inputs to config-maven BUILD-11500 Use self-hosted Artifactory for Maven actions Jun 2, 2026
@hedinasr
hedinasr force-pushed the feat/hnasr/BUILD-11500-configMavenArtifactoryInputs branch from b4665bb to 29e0585 Compare June 2, 2026 09:23
Remove repox-url and repox-artifactory-url from config-maven, build-maven,
and promote. Set ARTIFACTORY_URL to https://repox.dev.sonar.build/artifactory
in those actions; credentials still come from Vault as before.

Update README for config-maven, build-maven, and promote.

Breaking change: repox-url and repox-artifactory-url are no longer accepted
on config-maven, build-maven, or promote.
@hedinasr
hedinasr force-pushed the feat/hnasr/BUILD-11500-configMavenArtifactoryInputs branch from 29e0585 to 0314185 Compare June 2, 2026 09:24
Accidentally downgraded when restoring action files from an older branch commit.
Comment on lines 62 to 66
<releases>
<enabled>true</enabled>
<!-- no need to always check if new versions are available when executing a Maven plugin without specifying the version -->
<updatePolicy>interval:60</updatePolicy>
<updatePolicy>always</updatePolicy>
<checksumPolicy>fail</checksumPolicy>

@gitar-bot gitar-bot Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Quality: updatePolicy=always contradicts adjacent comment and adds overhead

In the plugin releases repository, updatePolicy was changed from interval:60 to always, but the comment directly above it still reads: "no need to always check if new versions are available when executing a Maven plugin without specifying the version". The code now does the exact opposite of what the comment states.

Functionally, always makes Maven check the remote repository for plugin updates on every single build. For a release (non-snapshot) plugin repository this is almost never desired — released artifacts are immutable, so the only effect is added network round-trips (and slower, less reliable builds) on each invocation. interval:60 (or daily) already balanced freshness against performance.

Suggested fix: either revert to interval:60/daily, or if always is intentional, update the comment to explain why frequent checks are now required. Note the <releases> repository block above (line 48) still uses interval:60, so this also introduces an inconsistency between the two repository definitions.

Revert to interval:60 to match the comment and the releases repository above.:

<releases>
  <enabled>true</enabled>
  <!-- no need to always check if new versions are available when executing a Maven plugin without specifying the version -->
  <updatePolicy>interval:60</updatePolicy>
  <checksumPolicy>fail</checksumPolicy>
</releases>

Was this helpful? React with 👍 / 👎

Comment thread build-maven/action.yml Outdated
@hedinasr
hedinasr force-pushed the feat/hnasr/BUILD-11500-configMavenArtifactoryInputs branch from 77bcec5 to 8bac5c3 Compare June 3, 2026 12:31
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

hedinasr added 9 commits July 22, 2026 12:45
…guard; derive vault URL from ARTIFACTORY_URL instead of hardcoding vault.dev
…RL; drop redundant prod-vault promote step
Reader credentials come from steps.artifactory, not steps.secrets
(Develocity-only). Empty auth caused 401 against repox-internal.
Also restore null-safe fromJSON for deploy vault outputs in build-maven.
Vault returns vault-SonarSource-... but Access token subjects are
vault-sonarsource-.... Edge DENIED LOGIN on case mismatch; Bearer worked.
@sonarqubecloud

Copy link
Copy Markdown

@hedinasr hedinasr closed this Jul 30, 2026
@hedinasr
hedinasr deleted the feat/hnasr/BUILD-11500-configMavenArtifactoryInputs branch July 30, 2026 08:11
@gitar-bot

gitar-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
CI failed: 1 pre-commit CI failure caused by a markdownlint rule violation (MD060) due to misaligned table pipes in README.md.

Overview

All 4 analyzed CI logs failed due to the same tooling error: the pre-commit hook markdownlint detected a style violation in README.md regarding table column alignment.

Failures

Markdownlint Table Style Violation (confidence: high)

  • Type: tooling
  • Affected jobs: 89161902486, 88903711508, 88921063466, 88933531658
  • Related to change: unclear
  • Root cause: The markdownlint check identified a violation of the MD060 rule ('Table column style') in README.md at line 243, where the table pipe does not align with the header.
  • Suggested fix: Correct the table pipe alignment in README.md around line 243 to satisfy the markdownlint configuration, or run pre-commit run --all-files locally to verify.

Summary

  • Change-related failures: 0 explicit code bugs; 1 formatting/linting check failure spanning all 4 jobs.
  • Infrastructure/flaky failures: 0 infrastructure or flaky failures.
  • Recommended action: Fix the table formatting in README.md so that the pre-commit workflow passes.
Code Review ⚠️ Changes requested 2 resolved / 3 findings

Transitions core Maven actions to the self-hosted Artifactory instance and resolves credential management issues. Updates to updatePolicy=always conflict with existing inline documentation and introduce unnecessary build overhead.

⚠️ Quality: updatePolicy=always contradicts adjacent comment and adds overhead

📄 config-maven/resources/settings.xml:62-66

In the plugin releases repository, updatePolicy was changed from interval:60 to always, but the comment directly above it still reads: "no need to always check if new versions are available when executing a Maven plugin without specifying the version". The code now does the exact opposite of what the comment states.

Functionally, always makes Maven check the remote repository for plugin updates on every single build. For a release (non-snapshot) plugin repository this is almost never desired — released artifacts are immutable, so the only effect is added network round-trips (and slower, less reliable builds) on each invocation. interval:60 (or daily) already balanced freshness against performance.

Suggested fix: either revert to interval:60/daily, or if always is intentional, update the comment to explain why frequent checks are now required. Note the <releases> repository block above (line 48) still uses interval:60, so this also introduces an inconsistency between the two repository definitions.

Revert to interval:60 to match the comment and the releases repository above.
<releases>
  <enabled>true</enabled>
  <!-- no need to always check if new versions are available when executing a Maven plugin without specifying the version -->
  <updatePolicy>interval:60</updatePolicy>
  <checksumPolicy>fail</checksumPolicy>
</releases>
✅ 2 resolved
Bug: secrets vault step lost url override after split

📄 build-maven/action.yml:146-160
When the Artifactory credential secrets were split out into the new artifactory vault step (lines 146-154), the url: https://vault.dev.sonar.build override was carried only to that new step. The original secrets step (lines 156-169) now has no url: input, so it falls back to the vault-action-wrapper default URL, while the adjacent artifactory step explicitly targets vault.dev.sonar.build.

If the wrapper's default URL is not the same instance, the two adjacent steps will resolve against different Vault instances within the same action run: Artifactory deploy credentials from vault.dev.sonar.build, but NEXT_TOKEN / SQC tokens / SIGN_KEY / PGP_PASSPHRASE from the default vault. That mismatch can break signing and analysis, or fetch the wrong secrets, on this branch. The combined step previously had a single url: https://vault.dev.sonar.build covering all of these secrets.

Suggested fix: set url: https://vault.dev.sonar.build on the secrets step as well (or remove it from both if the default is intended), so both steps consistently target the same Vault instance.

Edge Case: artifactory vault step may emit empty output for non-deploy builds

📄 build-maven/action.yml:146-154 📄 build-maven/action.yml:190-192
The new artifactory step requests zero secrets when inputs.deploy == 'false' (all three secret lines evaluate to ''), and it has no if: guard, so it always runs. Lines 190-192 then call fromJSON(steps.artifactory.outputs.vault).ARTIFACTORY_DEPLOY_USERNAME etc.

Previously these deploy credentials lived in the secrets step, whose output was always non-empty valid JSON because it also requested SIGN_KEY/PGP_PASSPHRASE. Now, for non-deploy builds (e.g. PR builds with deploy=false), the dedicated artifactory step may produce an empty vault output. If the wrapper emits '' rather than {} in that case, fromJSON('') would throw and fail the env evaluation of the build step for all non-deploy runs.

Please confirm vault-action-wrapper outputs {} (not empty string) when no secrets are requested; if not, add an if: guard on the artifactory step (e.g. only when deploy != 'false') and gate the dependent env vars accordingly.

🤖 Prompt for agents
Code Review: Transitions core Maven actions to the self-hosted Artifactory instance and resolves credential management issues. Updates to `updatePolicy=always` conflict with existing inline documentation and introduce unnecessary build overhead.

1. ⚠️ Quality: updatePolicy=always contradicts adjacent comment and adds overhead
   Files: config-maven/resources/settings.xml:62-66

   In the plugin releases repository, `updatePolicy` was changed from `interval:60` to `always`, but the comment directly above it still reads: "no need to always check if new versions are available when executing a Maven plugin without specifying the version". The code now does the exact opposite of what the comment states.
   
   Functionally, `always` makes Maven check the remote repository for plugin updates on every single build. For a *release* (non-snapshot) plugin repository this is almost never desired — released artifacts are immutable, so the only effect is added network round-trips (and slower, less reliable builds) on each invocation. `interval:60` (or `daily`) already balanced freshness against performance.
   
   Suggested fix: either revert to `interval:60`/`daily`, or if `always` is intentional, update the comment to explain why frequent checks are now required. Note the `<releases>` repository block above (line 48) still uses `interval:60`, so this also introduces an inconsistency between the two repository definitions.

   Fix (Revert to interval:60 to match the comment and the releases repository above.):
   <releases>
     <enabled>true</enabled>
     <!-- no need to always check if new versions are available when executing a Maven plugin without specifying the version -->
     <updatePolicy>interval:60</updatePolicy>
     <checksumPolicy>fail</checksumPolicy>
   </releases>

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change the behavior for this request:

Auto-apply Compact Unblock
gitar auto-apply:on         
gitar display:verbose         
gitar unblock         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants