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

Added additional_display_details to be default set. #236

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

hhovsepy
Copy link
Contributor

@hhovsepy hhovsepy commented Dec 1, 2023

Issue kiali/kiali#6665
Original feature: kiali/kiali#1567

Make sure that in helm installation the "additional_display_details" a set, similar to operator one: https://github.com/kiali/kiali-operator/blob/master/crd-docs/cr/kiali.io_v1alpha1_kiali.yaml#L8

Comment on lines +21 to +25
additional_display_details:
- annotation: kiali.io/api-spec
icon_annotation: kiali.io/api-type
title: API Documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really needed? Note the comment above that says, only those values used by the Helm Chart will be here. We only put stuff in here that the actual Helm charts reference and use. We don't set simple defaults here as there should be no need.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... I guess this isn't a hardcoded default in the server? I'm trying to think if we really want this in the values file or not - we never put defaults in here unless the charts actually use them, but in this case I don't see a way around having to put this in here.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think the difference here is that this setting is an array of items. So if we hardcode this default, but the user sets his own, that default list item will be overwritten. The user will need to know to add it back in if they want to see the Kiali API stuff. So, yes, I can see why we could put this in there, because it expicitly shows the user the kiali API spec list item and if they want their own, they can just add it to the list if they want to keep the kiali one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, this is not hardcoded values but says where to look the values

@hhovsepy hhovsepy merged commit 7d4f740 into kiali:master Dec 1, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants