Skip to content

VAULT-31185 & 31186/use identity token auth for Artifactory in Vault CE & Ent #31255

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.

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
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kporter101
Copy link

Description

What does this PR do?
Context: Artifactory username/ API keys are being deprecated in favor of identity tokens, which do not require a username.

  • Removes all references to Artifactory username
  • Replaces references to Artifactory token (API key) with Artifactory Bearer token (identity token)

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@kporter101 kporter101 requested a review from a team as a code owner July 11, 2025 18:19
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 11, 2025
@kporter101 kporter101 force-pushed the VAULT-31185/remove-artifactory-usename branch from c73a9a0 to 5b0c872 Compare July 11, 2025 18:23
Copy link

github-actions bot commented Jul 11, 2025

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Jul 11, 2025

Build Results:
All builds succeeded! ✅

@@ -44,8 +44,7 @@ jobs:
ENOS_VAR_vault_log_level: trace
ENOS_VAR_aws_ssh_private_key_path: ./support/private_key.pem
ENOS_VAR_tfc_api_token: ${{ secrets.TF_API_TOKEN }}
ENOS_VAR_artifactory_username: ${{ secrets.ARTIFACTORY_USER }}
ENOS_VAR_artifactory_token: ${{ secrets.ARTIFACTORY_TOKEN }}
ENOS_VAR_artifactory_token: ${{ secrets.ARTIFACTORY_BEARER_TOKEN }}
Copy link
Author

Choose a reason for hiding this comment

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

@ryancragun should this still be secrets. or something else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would say that it is only this for historical reasons. When it was initially written we used Github Actions secrets for everything. Now we prefer Github Actions secrets for public repos and the internal Vault for private repos.

So to answer: ideally we'd only have this secret set in hashicorp/vault and only use it when the workflow executes in the context of that repository. When it executes in hashicorp/vault-enterprise we probably ought to be retrieving it from Vault.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The way that we do it in test-run-enos-scenario-matrix maps exactly to how we ought to do it here.

@kporter101 kporter101 requested a review from a team as a code owner July 11, 2025 18:43
@kporter101 kporter101 requested a review from stuti-sr July 11, 2025 18:43
@kporter101 kporter101 force-pushed the VAULT-31185/remove-artifactory-usename branch 3 times, most recently from 6543d34 to 1f75fcc Compare July 14, 2025 19:25
@kporter101 kporter101 force-pushed the VAULT-31185/remove-artifactory-usename branch from 1f75fcc to 59d026a Compare July 14, 2025 20:00
Copy link
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

This looks good. Lets clean up the changelog and do some manual verification of the dev scenarios and merge it!

@@ -0,0 +1,3 @@
```release-note:change
secrets/artifactory: Use Artifactory identity token for auth, instead of username with API key
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're only changing the behavior of Vault CI, not the Vault binary itself, we don't need a changelog. Instead, use the pr/no-changelog label to get rid of failed check.

@@ -44,8 +44,7 @@ jobs:
ENOS_VAR_vault_log_level: trace
ENOS_VAR_aws_ssh_private_key_path: ./support/private_key.pem
ENOS_VAR_tfc_api_token: ${{ secrets.TF_API_TOKEN }}
ENOS_VAR_artifactory_username: ${{ secrets.ARTIFACTORY_USER }}
ENOS_VAR_artifactory_token: ${{ secrets.ARTIFACTORY_TOKEN }}
ENOS_VAR_artifactory_token: ${{ secrets.ARTIFACTORY_BEARER_TOKEN }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way that we do it in test-run-enos-scenario-matrix maps exactly to how we ought to do it here.

artifactory_token:
The artifactory token associated with your username. You'll need this if you wish to use
deb or rpm artifacts! You can create a token by logging into Artifactory via Okta.
The artifactory identity token associated with a service account. You'll need this if you wish to use
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we normalize the line length on these new changes?

@@ -1,13 +1,6 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

variable "artifactory_username" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🫗 for basic auth

Copy link
Author

Choose a reason for hiding this comment

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

🪦

@@ -73,12 +68,11 @@ module "artifact_metadata" {
}

data "enos_artifactory_item" "vault" {
username = var.artifactory_username
Copy link
Collaborator

Choose a reason for hiding this comment

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

Before merge we probably ought to test the dev scenarios with the package:rpm and package:deb variants to make sure they're working as expected.

@ryancragun
Copy link
Collaborator

I added the backport labels. Even though release testing always happens from main, we generally try and keep the enos and .github directories the same across all active branches.

@kporter101 kporter101 force-pushed the VAULT-31185/remove-artifactory-usename branch 2 times, most recently from 561a4dc to e02156c Compare July 22, 2025 18:38
@kporter101 kporter101 force-pushed the VAULT-31185/remove-artifactory-usename branch from e02156c to 4e85cd0 Compare July 23, 2025 19:14
Copy link

vercel bot commented Jul 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vault-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2025 8:53pm

Copy link
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

Lets update the descriptions in both dev scenarios and then merge!

Copy link
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

Image

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

Successfully merging this pull request may close these issues.

2 participants