Skip to content

feat(KFLUXUI-504): Add banner content yaml and its kustomization files #6630

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

Merged

Conversation

testcara
Copy link
Contributor

@testcara testcara commented Jun 10, 2025

KFLUXUI-500 aims to provide konflux banner to users.

The PR does:
supporting the single manual banner:

  • add the banner-content.yaml for each cluster under konflux-info
  • update its kustomize to ensure the its confimap could be generated
  • add README.md and github workflow validation support for banner content including schema & safe content
    supporting the multiple auto generated alerts:
  • add the auto-alerts directory and its fake kustomize
  • update genearl kustomize for auto-alerts
  • update README.md to explain how we can enjoy the auto-alerts.

Here is one video about how UI integrate with them:
https://drive.google.com/file/d/17X3qkgLD7aXWq_wnuteAWUu3WE7_J2M5/view

@openshift-ci openshift-ci bot requested review from arewm and tisutisu June 10, 2025 03:10
Copy link
Member

@gbenhaim gbenhaim left a comment

Choose a reason for hiding this comment

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

Please move the content of https://github.com/testcara/konflux-banner into a directory in this repo (under the components directory)

Please deploy those configmap to the konflux-info namespace since its purpose is to store information which is accessible by all authenticated users.

Thanks !

@gbenhaim
Copy link
Member

I really liked the implementation because it will also be possible to generate those confimaps with the banner details dynamically based on alerts.

I left a comment above about the way it should be deployed.

Thank you for working on this.

@gbenhaim
Copy link
Member

I have a question, does each banner stored in its own configmap? I would like to also generate banners dynamically, it means that we would need the UI to read multiple configmaps with the banner details from the cluster.
Is it possible?

cc @sahil143

@testcara
Copy link
Contributor Author

testcara commented Jun 10, 2025

I have a question, does each banner stored in its own configmap? I would like to also generate banners dynamically, it means that we would need the UI to read multiple configmaps with the banner details from the cluster. Is it possible?

cc @sahil143

@gbenhaim Thank you for the review and quick response.

  • does each banner stored in its own configmap -> yeah. it would be

  • I would like to also generate banners dynamically -> based on the current design, users just need to submit one pure yaml PR manually to specify the banner content, see the example PR: https://github.com/testcara/konflux-banner/pull/5/files . Once the PR is merged. The corresponding configmap would be refreshed in real time. but it is also good to update the yaml by some automation. CI for the banner would test its schema and content to ensure the update is validate.

  • it means that we would need the UI to read multiple configmaps with the banner details from the cluster. Is it possible?
    -> One cluster just have one banner configmap. so UI just fetch its own banner configmap then show its content.

@testcara
Copy link
Contributor Author

testcara commented Jun 10, 2025

Please move the content of https://github.com/testcara/konflux-banner into a directory in this repo (under the components directory)

Please deploy those configmap to the konflux-info namespace since its purpose is to store information which is accessible by all authenticated users.

Thanks !

@gbenhaim could you help to double-confirm whether i got your suggestion correctly?

  • move the github.com/testcara/konflux-banner/ as konflux-banner-content component to infra-deplouments repo
  • delete the testcara/konflux-banner repo
  • update the konflux-banner component properly:
    • changing konflux-banner namespace to konflux-info
    • changing repoURL to the new one

Thank you for the great help. :-)

@gbenhaim
Copy link
Member

gbenhaim commented Jun 10, 2025

Please move the content of https://github.com/testcara/konflux-banner into a directory in this repo (under the components directory)
Please deploy those configmap to the konflux-info namespace since its purpose is to store information which is accessible by all authenticated users.
Thanks !

@gbenhaim could you help to double-confirm whether i got your suggestion correctly?

  • move the github.com/testcara/konflux-banner/ as konflux-banner-content component to infra-deplouments repo

You can use the existing konflux-info argo app and component folder, no need to create a new one.

  • delete the testcara/konflux-banner repo

  • update the konflux-banner component properly:

    • changing konflux-banner namespace to konflux-info
    • changing repoURL to the new one

Thank you for the great help. :-)

The rest is correct.

I would also want to get a change in the ui so it would know how to read multiple configmaps from the cluster. Dynically generated configmap would be creates using automation directly in the cluster and not using prs.

@testcara testcara marked this pull request as draft June 10, 2025 16:53
@testcara testcara force-pushed the konflux-banner-appsets branch 5 times, most recently from aa3b9de to 46133f0 Compare June 13, 2025 07:51
@testcara testcara marked this pull request as ready for review June 13, 2025 07:52
@openshift-ci openshift-ci bot requested review from ralphbean and simonbaird June 13, 2025 07:52
@testcara
Copy link
Contributor Author

@gbenhaim hi, Gal, i refreshed the PR, could you help to review again? BTW, the description includes what the PR does or not covered, hope it makes the review a bit easier. Thank you for the help.

@testcara testcara changed the title feat(KFLUXUI-504): Add apps and kustomization for konflux-banner feat(KFLUXUI-504): Add banner content yaml and its kustomization files Jun 13, 2025
@testcara testcara marked this pull request as draft June 26, 2025 07:19
@testcara testcara force-pushed the konflux-banner-appsets branch from 46133f0 to f63dc77 Compare June 26, 2025 07:20
@testcara testcara force-pushed the konflux-banner-appsets branch from f63dc77 to 30c6b01 Compare June 26, 2025 07:29
@testcara testcara marked this pull request as ready for review June 26, 2025 07:34
@openshift-ci openshift-ci bot requested review from elsony and johnmcollier June 26, 2025 07:34
@testcara testcara marked this pull request as draft July 1, 2025 07:46
@testcara testcara force-pushed the konflux-banner-appsets branch from d093746 to 930461d Compare July 1, 2025 08:02
@testcara testcara marked this pull request as ready for review July 1, 2025 09:14
@testcara
Copy link
Contributor Author

testcara commented Jul 1, 2025

@gbenhaim i tried to support the two kinds of banners now, see the https://drive.google.com/file/d/17X3qkgLD7aXWq_wnuteAWUu3WE7_J2M5/view. looking forward to any feedback. thank you.

@openshift-ci openshift-ci bot added the approved label Jul 7, 2025
@gbenhaim
Copy link
Member

gbenhaim commented Jul 7, 2025

/approve
/lgtm

Copy link

openshift-ci bot commented Jul 7, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gbenhaim, testcara

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kind: ConfigMap
metadata:
labels:
konflux-auto-alert: "true"
Copy link
Contributor Author

@testcara testcara Jul 7, 2025

Choose a reason for hiding this comment

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

i would change the label to recommended way like 'app.kubernetes.io/name: my-app'

@openshift-merge-bot openshift-merge-bot bot merged commit 6a72b29 into redhat-appstudio:main Jul 7, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants