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

feat: Open id connect resource view #3739

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

KonradPietocha
Copy link
Contributor

@KonradPietocha KonradPietocha commented Mar 4, 2025

Description

Changes proposed in this pull request:

  • openid-connect.yaml file has been added
  • The new file was applied to add OpenIDConnect to side nav

Related issue(s)
Closes #3651

Definition of done

  • The PR's title starts with one of the following prefixes:
    • feat: A new feature
    • fix: A bug fix
    • docs: Documentation only changes
    • refactor: A code change that neither fixes a bug nor adds a feature
    • test: Adding tests
    • revert: Revert commit
    • chore: Maintainance changes to the build process or auxiliary tools, libraries, workflows, etc.
  • Related issues are linked. To link internal trackers, use the issue IDs like backlog#4567
  • Explain clearly why you created the PR and what changes it introduces
  • All necessary steps are delivered, for example, tests, documentation, merging

@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 4, 2025
@KonradPietocha KonradPietocha changed the title Open id connect resource view feat: Open id connect resource view Mar 4, 2025
@mrCherry97 mrCherry97 self-assigned this Mar 4, 2025
Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

  • This informations should be displayed on the list view
    image

  • Create Form should contains all fields which user can fill for spec

  • editing should be limited only to not managed resources (not labeled by Kyma Infrastructure Manager)

Comment on lines 52 to 58
presets: |-
- name: template
default: true
value:
metadata:
annotations:
authentication.gardener.cloud/class: garden
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's necessary

Comment on lines 44 to 51
- source: spec
name: Specification JSON
widget: CodeViewer
language: "'json'"
- source: metadata
name: Metadata
widget: CodeViewer
language: "'json'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Metadata is in the header, spec is in Panel, it's not so necessary.

Comment on lines 24 to 26
header:
- source: metadata.name
name: Name
Copy link
Contributor

Choose a reason for hiding this comment

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

This also could be removed.

Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

You are adding this file in new directory, so to build it in Busola you need to add it into extensions.json files for dev, stage and prod

source: spec.clientID
form: |-
- path: spec.issuerURL
visibility: $not((metadata.labels."operator.kyma-project.io/managed-by") = "infrastructure-manager")
Copy link
Contributor

Choose a reason for hiding this comment

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

this couldn't be done as visibility, because the user still can mess up in YAML, this should be done in disableEdit in general section.

Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

Something is wrong with description in SimpleList, if it's possible please remove description (probably taken automatically from CRD) or open an issue to fix this misalignment's
Screenshot 2025-03-06 at 09 54 53

- false
- path: spec.supportedSigningAlgs
visibility: $not((metadata.labels."operator.kyma-project.io/managed-by") = "infrastructure-manager")
overwrite: false
Copy link
Contributor

Choose a reason for hiding this comment

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

What this flag is doing? I don't think it's necessary.

Copy link
Contributor Author

@KonradPietocha KonradPietocha Mar 6, 2025

Choose a reason for hiding this comment

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

When visibility is set to false I saw that all fields is somehow overwrite by this and it detects that there are some changes (probably if visibility is false all fields are clearing). This flag prevents this changes but will be not needed if I'll remove visibility and use disableEdit like you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenIDConnect resource view
3 participants