feat(ISV-5447): add multi-arch and sha info to release note images

[wcheang]

This is ready for review, but depends on konflux-ci/release-service-utils#333 to be merged first so that the multiarch field is populated before merge.

[openshift-ci]

Hi @wcheang. Thanks for your PR.

I'm waiting for a konflux-ci member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[johnbieren]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

[wcheang]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

Thanks @johnbieren, added the schema.

[wcheang]

My PTO is starting tomorrow, so @jedinym from my team will be taking over getting this PR merged.

[mmalina]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

[mmalina]

Do not merge. The image needs to be updated with konflux-ci/release-service-utils#333 first.

For the future, please create the PR as a Draft or at least create a thread as a reminder that something still needs to be done. Otherwise it can lead to premature merge as just happened here: hacbs-release/app-interface-deployments#245

[johnbieren]

Moved to draft

[johnbieren]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

The releaseNotes stuff is unique. We don't overwrite. We =+ https://github.com/konflux-ci/release-service-catalog/blob/development/tasks/populate-release-notes-images/populate-release-notes-images.yaml#L111
A user could define the same stuff the task is adding for them. I don't know why they'd want to, but they could. So, if a field could be provided by the user, I think it should be in the schema

[johnbieren]

I replied on the ticket, but why are we adding new fields to the advisories? Shouldn't this be driven by prodsec? Maybe it already is, but I haven't seen it

[mmalina]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

The releaseNotes stuff is unique. We don't overwrite. We =+ https://github.com/konflux-ci/release-service-catalog/blob/development/tasks/populate-release-notes-images/populate-release-notes-images.yaml#L111 A user could define the same stuff the task is adding for them. I don't know why they'd want to, but they could. So, if a field could be provided by the user, I think it should be in the schema

I'm not completely sure I buy this argument - it seems to me that being theoretically able to set this already in the RPA is more of a side effect. I'm not even sure if that should be allowed at all - it should say what was actually included in that release, right? So users shouldn't really be able to say that something extra is being released too when it's not. Or am I missing something?

[mmalina]

I replied on the ticket, but why are we adding new fields to the advisories? Shouldn't this be driven by prodsec? Maybe it already is, but I haven't seen it

Good point. I completely missed the fact that this will also add these fields to the resulting advisory json.

[johnbieren]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

The releaseNotes stuff is unique. We don't overwrite. We =+ https://github.com/konflux-ci/release-service-catalog/blob/development/tasks/populate-release-notes-images/populate-release-notes-images.yaml#L111 A user could define the same stuff the task is adding for them. I don't know why they'd want to, but they could. So, if a field could be provided by the user, I think it should be in the schema

I'm not completely sure I buy this argument - it seems to me that being theoretically able to set this already in the RPA is more of a side effect. I'm not even sure if that should be allowed at all - it should say what was actually included in that release, right? So users shouldn't really be able to say that something extra is being released too when it's not. Or am I missing something?

I am referring to adding it to the Release data section, not ReleasePlanAdmission

[mmalina]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

The releaseNotes stuff is unique. We don't overwrite. We =+ https://github.com/konflux-ci/release-service-catalog/blob/development/tasks/populate-release-notes-images/populate-release-notes-images.yaml#L111 A user could define the same stuff the task is adding for them. I don't know why they'd want to, but they could. So, if a field could be provided by the user, I think it should be in the schema

I'm not completely sure I buy this argument - it seems to me that being theoretically able to set this already in the RPA is more of a side effect. I'm not even sure if that should be allowed at all - it should say what was actually included in that release, right? So users shouldn't really be able to say that something extra is being released too when it's not. Or am I missing something?

I am referring to adding it to the Release data section, not ReleasePlanAdmission

What difference does it make? I understand that if you create a manual Release then you know the snapshot and so you can enter correct data about images in the snapshot into the Release data section, but those would then be duplicated in our task anyway. So not sure what the point would be.

[johnbieren]

If you are adding things to releaseNotes the schema needs to be updated https://github.com/konflux-ci/release-service-catalog/blob/development/schema/dataKeys.json

This is added during the release pipeline. I thought the schema only covers what's there at the start, but apparently I was wrong. What's the purpose of having releaseNotes.content.images there? It will never be validated, no?

The releaseNotes stuff is unique. We don't overwrite. We =+ https://github.com/konflux-ci/release-service-catalog/blob/development/tasks/populate-release-notes-images/populate-release-notes-images.yaml#L111 A user could define the same stuff the task is adding for them. I don't know why they'd want to, but they could. So, if a field could be provided by the user, I think it should be in the schema

I'm not completely sure I buy this argument - it seems to me that being theoretically able to set this already in the RPA is more of a side effect. I'm not even sure if that should be allowed at all - it should say what was actually included in that release, right? So users shouldn't really be able to say that something extra is being released too when it's not. Or am I missing something?

I am referring to adding it to the Release data section, not ReleasePlanAdmission

What difference does it make? I understand that if you create a manual Release then you know the snapshot and so you can enter correct data about images in the snapshot into the Release data section, but those would then be duplicated in our task anyway. So not sure what the point would be.

Because you want an image in your advisory that is not part of your snapshot or isn't mapped or something. I don't know. I am just saying it is a possible use case so it should be in the schema

[jedinym]

I replied on the ticket, but why are we adding new fields to the advisories? Shouldn't this be driven by prodsec? Maybe it already is, but I haven't seen it

@johnbieren If I understand correctly, the new fields in the advisory are not the intention here. They are only a side effect of adding additional data for the update-component-sbom task. I will refactor this PR to avoid that issue.

[johnbieren]

I replied on the ticket, but why are we adding new fields to the advisories? Shouldn't this be driven by prodsec? Maybe it already is, but I haven't seen it

@johnbieren If I understand correctly, the new fields in the advisory are not the intention here. They are only a side effect of adding additional data for the update-component-sbom task. I will refactor this PR to avoid that issue.

Thank you!

[jedinym]

@mmalina @johnbieren I refactored the PR to avoid changing the advisories. It's ready for review, thanks. I'd like to merge this before #627.