Skip to content

engineering: Enable password-access to ORAS behind flag#91

Merged
ayaegashi merged 9 commits into
mainfrom
ayaegashi/enable-pw-oras
Aug 6, 2025
Merged

engineering: Enable password-access to ORAS behind flag#91
ayaegashi merged 9 commits into
mainfrom
ayaegashi/enable-pw-oras

Conversation

@ayaegashi
Copy link
Copy Markdown
Contributor

🔍 Description

This PR adds the code to pull from container registries which require username:password credentials to access. This is not officially supported by Trident, so it is hidden behind a flag.

🤔 Rationale

Adding this code so that if in the future we want to support password-protected container registries, we have the logic readily available.

📝 Testing

Since this is hidden behind a flag, this code is not tested in our pipelines. To manually test this code, do the following:

  • Create an ACR (follow the steps here: https://learn.microsoft.com/en-us/azure/container-registry/container-registry-get-started-azure-cli)
  • In Azure Portal, navigate to the newly created ACR. On the left-hand panel, navigate to Settings > Access keys.
  • Click the check mark for "Admin user". This will then reveal a username and two passwords.
  • Store these credentials in ~/.docker/config.json in the MOS. One way to do this is to run echo <password> | docker login <container registry>.azurecr.io -u <username> --password-stdin in the post-install script.
  • Run Trident as usual, commenting out the #[cfg(feature = "grpc-dangerous")] tags in reader.rs.

@ayaegashi ayaegashi requested a review from a team as a code owner August 5, 2025 21:57
@ayaegashi ayaegashi changed the title enable password access behind flag engineering: Enable password-access to ORAS behind flag Aug 5, 2025
@ayaegashi
Copy link
Copy Markdown
Contributor Author

/AzurePipelines run

@ayaegashi
Copy link
Copy Markdown
Contributor Author

/AzurePipelines run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/osimage/cosi/reader.rs Outdated
use tokio::runtime::Runtime;
use url::Url;

#[cfg(feature = "grpc-dangerous")]
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.

sounds like this should get its own flag, this is not related to grpc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh... I might be a little confused is there more than one "dangerous" flag?

Copy link
Copy Markdown
Contributor Author

@ayaegashi ayaegashi Aug 5, 2025

Choose a reason for hiding this comment

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

Using #[cfg(feature = "dangerous-options")] since this seems to be related to password security...

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.

dangerous-options is the one that gets used for local builds:

cargo build --release --features dangerous-options

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see. Just to clarify, should this ORAS functionality be separated under another feature flag then? Or is it okay since it will not go into pipelines and release RPMs?

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.

dangerous-options is ok, the comment was mostly about not using the grps one :)

Comment thread src/osimage/cosi/reader.rs Outdated
impl ReadSeek for Cursor<Vec<u8>> {}

#[cfg(feature = "dangerous-options")]
const DOCKER_CONFIG_FILE_PATH: &str = "/root/.docker/config.json";
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.

We could use $HOME or the home_dir method instead of hard-coding /root

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed, env::home_dir() and joining with this constant below

@ayaegashi ayaegashi merged commit 524eac6 into main Aug 6, 2025
9 checks passed
@Britel Britel deleted the ayaegashi/enable-pw-oras branch September 29, 2025 20:08
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.

3 participants