Conversation
danielorihuela
changed the title
danielorihuela
force-pushed
the
feat/global-registry-support
branch
from
ec225bf to
0aa496f
Compare
danielorihuela
added
k8s-extended-e2e
danielorihuela
force-pushed
the
feat/global-registry-support
branch
from
0aa496f to
6948f2f
Compare
This reverts commit 701925b.
danielorihuela
force-pushed
the
feat/global-registry-support
branch
from
6948f2f to
3bcd04d
Compare
danielorihuela
changed the base branch from
main
to
feat/remove-oci-auth-support-from-agent-types
danielorihuela
force-pushed
the
feat/global-registry-support
branch
from
516b356 to
08ea56e
Compare
| #[serde(default)] | ||
| pub self_update: SelfUpdateConfig, | ||
|
|
||
| /// Oci configuration (used for AC and agents packages) |
There was a problem hiding this comment.
is the self update config modification missing ?
There was a problem hiding this comment.
but perhaps a separate pr is better
There was a problem hiding this comment.
Yes...
I managed to have it working, then I merged @paologallinaharbur changes.
I'll work on the internal struct on a different PR. But at least, with this one, I think @alvarocabanas could take his task, if he wants to.
There was a problem hiding this comment.
I mean. I'm aware we have two configs for the same thing now. I just didn't want to do more changes in this PR.
There was a problem hiding this comment.
I managed to have it working, then I merged @paologallinaharbur changes.
🤣
gsanchezgavier
left a comment
gsanchezgavier
left a comment
There was a problem hiding this comment.
thanks! i just lefts some minor comments
| .map_err(|err| OCIDownloaderError(err.to_string())) | ||
| } | ||
|
|
||
| fn download_package_artifact( |
There was a problem hiding this comment.
would make sense to change this fn to something that clearly states that the registry will be override?
| /// Returns the [Reference] after verifying its signature. The reference always includes the `digest` to | ||
| /// assure it is the same reference whose signature was verified. | ||
| /// It returns an error if signature verification fails. | ||
| fn verified_package_signature_reference( |
| } | ||
| } | ||
|
|
||
| fn reference_with_registry(reference: &Reference, registry: &str) -> Reference { |
There was a problem hiding this comment.
nit: override_registry(
| let agent_control_package_manager = OCIPackageManager::new( | ||
| OCIArtifactDownloader::new( | ||
| oci_client.clone(), | ||
| self.bootstrap_config.oci.registry.clone(), |
There was a problem hiding this comment.
o this pr is already taking car of the ac updater, you might miss to remove the config from it and perhaps something inside the updater itself
|
|
||
| #[derive(Debug, Deserialize, Serialize, Default, Clone, PartialEq)] | ||
| pub struct BearerAuth { | ||
| pub token: String, |
There was a problem hiding this comment.
at some point we are going to fetch this from other secured places but i think we can extend this without breaking
There was a problem hiding this comment.
I'm not sure about that. From where are we going to take that in the future?
We don't support secrets in the agent control config. At least for now.
| oci: | ||
| auth: | ||
| bearer: | ||
| token: "token" |
There was a problem hiding this comment.
So we go for bearer: token: at the end.. ok.
4 participants
Add support for a single configurable registry.
Context
We decided after some discussions to simplify the solution. Initially, we wanted to have a global value to configure the registry, that could then be override in an agent-per-agent basis. This was too complex. We decided to only give support for using one registry at a time, which can be configured.
This PR does that. It adds a global option in agent control to configure the registry, and uses that option to decide from where to download the packages. This is not a final solution, but a "shortcut" to unblock following tasks.
Technical details
registrywas removed from agent typesWe no longer give support to configure the registry on the agent type configuration. It must be configured from the agent control config. In other words, all packages must be downloaded from the same registry.
shortcut solution
Removing the
registryimpacts several parts of the codebase. It's not a simple removal of a field. To avoid blocking other tasks, we are moving ahead with this temporal solution. Basically, we "patch" the registry before doing any operation with the oci downloader.base.ioregistry when templating the references that's not used but complies with the format of a referenceIn a different PR, we will modify the code to reflect the correct new structure.