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

fix: add redhat registry logo #64

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

dgolovin
Copy link
Collaborator

@dgolovin dgolovin commented Feb 29, 2024

Fix #38.

image

Signed-off-by: Denis Golovin <[email protected]>
@dgolovin dgolovin requested a review from a team as a code owner February 29, 2024 08:10
@dgolovin
Copy link
Collaborator Author

There seems to be an issue with spacing between icon and registry name for this kind of entries.

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

IMHO we should include the png image as a resource

like https://github.com/containers/podman-desktop/blob/main/extensions/registries/src/extension.ts#L22

and https://github.com/containers/podman-desktop/blob/main/extensions/registries/src/extension.ts#L70-72

so you don't need to read the file, it's included in the .js file
also if the file is not found, it raises compilation error while in your case it'll be deferred to the runtime

@dgolovin
Copy link
Collaborator Author

I was looking for that, but missed it. I'll update the PR.

@vrothberg
Copy link
Collaborator

There seems to be an issue with spacing between icon and registry name for this kind of entries.

Is that something to fix in Podman Desktop or the extension?

Is there a way we can change the name to "Red Hat Container Registry"?

Copy link
Member

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

Shouldn't we have a more friendly name ?

@benoitf
Copy link
Collaborator

benoitf commented Feb 29, 2024

I've added the name change in the GitHub UI suggested change

@vrothberg
Copy link
Collaborator

I've added the name change in the GitHub UI suggested change

Looks great, thank you!
image

The only very minor thing that strikes my eyes is the user name. It looks cryptic as it's machine-generated. Not sure how we could beautify that.

@benoitf
Copy link
Collaborator

benoitf commented Feb 29, 2024

  • do you have a way to provide metadata to make it shorter as well ? (like -pd vs -podman-desktop) (probably something we can tune on our side)

  • can we have emails rather than digits on registry.redhat.io side ? like username "[email protected]"

  • we could enrich Podman Desktop API to include a username alias. So it could display "[email protected]" but if you try to edit it'll display you real digit-podman-desktop value but at least it would render more nicely. One issue is that for now the data are stored in auth.json so there won't be issue in Podman if we add extra fields in auth.json ?

like

{
        "auths": {
                "quay.io": {
                        "alias": "[email protected]",
                        "auth": "encoded-value"
                }
        }
}

@vrothberg
Copy link
Collaborator

I am unable to answer the questions but am sure that @dgolovin knows. I like the idea of displaying the email address used in SSO.

@benoitf
Copy link
Collaborator

benoitf commented Feb 29, 2024

@vrothberg do you know if podman will ignore extra fields that we may store in containers/auth.conf file ?

@vrothberg
Copy link
Collaborator

@vrothberg do you know if podman will ignore extra fields that we may store in containers/auth.conf file ?

It's JSON, so it very likely ignores unknown fields. But it's dangerous territory as it may break at any time in the future if we exploit the auth.conf for another purpose.

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

I'm also fine to have the png import in a follow up PR

@dgolovin
Copy link
Collaborator Author

dgolovin commented Mar 1, 2024

@benoitf There is no way to control user name, I call API with request to create 'podman-desktop' service account and it returns json with username/password. It looks like user name is generated as ${organizationID}|${requested-service-account-name}.

@dgolovin
Copy link
Collaborator Author

dgolovin commented Mar 1, 2024

BTW icon and name for the registry does not survive podman desktop restart podman-desktop/podman-desktop#6218

@dgolovin
Copy link
Collaborator Author

dgolovin commented Mar 1, 2024

Created follow up issue #68

@dgolovin dgolovin merged commit 597b2d9 into redhat-developer:main Mar 1, 2024
3 checks passed
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.

Add logo for Red Hat Registry
4 participants