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.

Sign up for GitHub

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

ansible: Include console.redhat.com password in image-upload secrets #596

Merged
merged 1 commit into from
Mar 11, 2024
Merged

ansible: Include console.redhat.com password in image-upload secrets #596

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ansible/roles/tasks-systemd/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,17 @@
[container.secrets]
# these are *host* paths, this is podman-remote
# FIXME: Split the upload/download secrets
# secret from issue-scan for image refreshes
image-upload=[
'--volume=/var/lib/cockpit-secrets/tasks/s3-keys/:/run/secrets/s3-keys:ro',
'--env=COCKPIT_S3_KEY_DIR=/run/secrets/s3-keys',
# password for console.redhat.com when image-create'ing rhel4edge
'--volume=/var/lib/cockpit-secrets/tasks/crc_passwd:/run/secrets/crc_passwd:ro',
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, this has nothing to do with uploading images, but rather downloading the prerequisites?

How do you feel about making a separate secret for that? Too much work? If so, I'd prefer to have a more explicit comment here...

Copy link
Member

Choose a reason for hiding this comment

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

For example, I might imagine a "rhel" secret which:

  • gives us download-only access for rhel images on the linode infra (which is another topic on the pilot board)
  • gives us CRC

We would then provide that secret for any job that gets in the rhel queue. tests-scan already needs to know about that distinction anyway...

Copy link
Member

Choose a reason for hiding this comment

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

This would even be nice because then we could make our check for "which queue does this belong in" equal to "does this have the rhel secret set on it?"

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it's part of the image-create task; the alternative is to make it a new secret and add it to bots' issues-scan, but they aren't really independent from a "which kind of task you want to run" POV. I don't mind either way, but I would organize them by-purpose. In fact, I'm actually inclined to rename them to "image-refresh" and "run-test".

Copy link
Member Author

Choose a reason for hiding this comment

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

The "rhel" queue was originally meant for things that need to run inside of the Red Hat VPN. That got somewhat muddied/partially obsoleted with putting all our images into S3, as that now depends on ownership of the token instead of being in the VPN. That leaves building rhel* images which get sent to the "rhel" queue, as they do need to access the internal package mirror.

Curiously, building rhel4edge doesn't -- it accesses console.redhat.com, which is not restricted to the RH VPN (it's a public service). So it's conceptually wrong to put these in to the "rhel" queue, they can be built on public infra as well (as long as you have the password).

Copy link
Member

Choose a reason for hiding this comment

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

The fact remains that rhel4edge is only ever going to refresh on hosts behind the VPN because issue-scan:172 says

def queue_task(channel, body):
    queue = 'rhel' if contains_any(body['job']['context'], REDHAT_TASKS) else 'public'
    channel.basic_publish('', queue, json.dumps(body),
                          properties=pika.BasicProperties(priority=distributed_queue.MAX_PRIORITY))

Copy link
Member Author

Choose a reason for hiding this comment

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

We talked about the options in chat:

  • New "rhel" or "rhel-contents" secret, and only add that to issue-scan: weird as tests also can get RHEL contents via the "image-download" secret.
  • Merge CRC into "image-download" and possibly rename it to "rhel-contents": bad as tests would then get access to CRC, but don't need to
  • New "crc" secret: would be strictly tied to "image-upload" in tests-scan, and also conceptually bad: {test,issue}-scan shouldn't know about such "special case" tokens. It knows about purposes/tasks (image-refresh or test) only.
  • Attaching it to the "rhel" queue: technically false as CRC is not RH-internal infra, and also tests don't need access to it.

In summary, I think adding it to image-upload is conceptually the cleanest approach. I'm open to renaming it to image-create.

'--env=COCKPIT_CRC_PASSWORD=/run/secrets/crc_passwd',
]
# secret from tests-scan for downloading RHEL images
image-download=[
# FIXME: create a new "download only" S3 token
'--volume=/var/lib/cockpit-secrets/tasks/s3-keys/:/run/secrets/s3-keys:ro',
'--env=COCKPIT_S3_KEY_DIR=/run/secrets/s3-keys',
]
Expand Down