Skip to content
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

Add registry sidecar support #146

Closed
wants to merge 19 commits into from
Closed

Conversation

jesusbv
Copy link
Contributor

@jesusbv jesusbv commented Feb 16, 2024

No description provided.

@jesusbv jesusbv self-assigned this Feb 16, 2024
@rjschwei rjschwei marked this pull request as draft February 16, 2024 16:40
@rjschwei
Copy link
Contributor

Switching to draft, we are still in the scoping phase of the project.

Set registry FQDN as registry-framework.susecloud.net in SMT
Use SMT object to get registry FQDN

Update tests
@jesusbv jesusbv force-pushed the add-registry-sidecar-support branch from 46f3eba to f3cf6c0 Compare March 1, 2024 10:02
When login with docker/podman, an auths token is written
to the config file

- Set that config file with the auth token without running login command
lib/cloudregister/smt.py Outdated Show resolved Hide resolved
@jesusbv jesusbv requested a review from schaefi March 12, 2024 09:05
auth_token = base64.b64encode('{username}:{password}'.format(
username=username,
password=password
).encode()).decode()
Copy link
Contributor

Choose a reason for hiding this comment

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

We carry the username and password information as base64 for obfuscation reasons right ? not for security, right ? Just asking because there is no security with b64encode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is not because of security, it is because we have to, docker|podman dont like anything that is not base64

> cat ${XDG_RUNTIME_DIR}/containers/auth.json
{
	"auths": {
		"registry.suse-local.com:5000": {
			"auth": "SCC_foo:password_bar"
		}
	}
}
> podman login  --tls-verify=false
Error: get credentials: 1 error occurred:
	* illegal base64 data at input byte 72
> cat ~/.docker/config.json
{
        "auths": {
                "registry.suse-local.com:5000": {
                        "auth": "SCC_foo:password_bat"
                }
        }
}
> docker login registry.suse-local.com:5000
WARNING: Error loading config file: ~/.docker/config.json: illegal base64 data at input byte 3
Username:

from the podman doc

[...] Podman creates ${XDG_RUNTIME_DIR}/containers/auth.json (if the file does not exist) and then stores the username and password from STDIN as a base64 encoded string in it

from the docker doc

 [...]
 If none of these binaries are present, it stores the credentials (i.e. password) in base64 encoding in the config files described above

- Add registry sidecar RMT and SUSE registry URLs
  to podmand and docker config files
- Add toml to spec file
- Update tests to reflect the change
@jesusbv jesusbv force-pushed the add-registry-sidecar-support branch from 4ab64d5 to 243d380 Compare March 14, 2024 10:55
@jesusbv jesusbv force-pushed the add-registry-sidecar-support branch from f216223 to 243d380 Compare April 17, 2024 17:53
- For podman set registry outside the unqualified search registries
  in order to be able to inform the user when credentials expired and
  a reauth is needed

- For docker do not add registry-<csp>.susecloud.net to the search
  docker does not suppoert auth search, and the registry will not
  show free repos

- Update tests
@jesusbv jesusbv marked this pull request as ready for review April 23, 2024 16:50
Copy link
Contributor

@schaefi schaefi left a comment

Choose a reason for hiding this comment

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

A few comments from my side

lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
- Remove duplication
- Add check for Docker
- Log if configuration files not found
@jesusbv
Copy link
Contributor Author

jesusbv commented May 16, 2024

Closing in favor of #157

@jesusbv jesusbv closed this May 16, 2024
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.

None yet

4 participants