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

App page secrets error #4021

Merged
merged 17 commits into from
Sep 21, 2023
Merged

App page secrets error #4021

merged 17 commits into from
Sep 21, 2023

Conversation

AlinaGoaga
Copy link
Contributor

@AlinaGoaga AlinaGoaga commented Sep 19, 2023

Closes #3843

What changed?
For helm releases, do not try and read secret if spec.kubeConfig has been set.

Why was this change made?
On the Applications Page, users would get several errors related to the secret not being present.
get helmrelease inventory: secrets "sh.helm.release.v1.grafana-agent-logs.v7" not found.
This is not relevant for cases in which kubeConfig is used.

@AlinaGoaga AlinaGoaga marked this pull request as ready for review September 19, 2023 10:04
core/server/inventory.go Outdated Show resolved Hide resolved
Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

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

Nice clean up too!

core/server/inventory_test.go Outdated Show resolved Hide resolved
@foot
Copy link
Contributor

foot commented Sep 20, 2023

Lets try adding a little message to the UI too, some little admonition like..

Note
spec.Kubeconfig is set on this HelmRelease. Details about reconciled objects are not available.

@AlinaGoaga
Copy link
Contributor Author

Hi @mmoulian, we're got an info alert in this PR. I can't seem to be able to find something similar in the designs. Would you mind having a look at the styles and let me know which colors you'd prefer / add it to Figma? Below is how it looks atm.

Thank you

Screenshot 2023-09-20 at 15 38 59

Screenshot 2023-09-20 at 15 38 51

@mmoulian
Copy link

Hi @AlinaGoaga! Searching through our archive files I found this solution. We did it for alerts and errors, but I'm not sure if the alert in PR is the same type or needs a distinction.

differents states

Link Figma: https://www.figma.com/file/IVHnM9iyeFWpd11evtY8ux/Weave-GitOps?type=design&node-id=19837%3A54217&mode=design&t=TuqsDZiQ4rVgsarD-1

This solution is applied in our Demo on a page like this:
Captura de pantalla 2023-09-20 a las 16 10 38

Let me know if it help you!

@AlinaGoaga
Copy link
Contributor Author

Thanks @mmoulian, yup I know these ones, we need one in blue for the info box please (light and dark)

@AlinaGoaga AlinaGoaga merged commit 565e1d4 into main Sep 21, 2023
15 checks passed
@AlinaGoaga AlinaGoaga deleted the app-page-secrets-error branch September 21, 2023 13:21
@mmoulian
Copy link

@AlinaGoaga I made the design here. ..It is very similar to your proposal. You can see it in dark or light mode (see movie).
Things to keep in mind:
-The corners are rounded 8 px
-Texts 14 px / line height 16 px
-I added these colors to our Variable Design System (See image)

https://github.com/weaveworks/weave-gitops/assets/95616291/36d928a5-9d94-4b3e-8c4d-9c57ac8056c3
https://www.figma.com/file/IVHnM9iyeFWpd11evtY8ux/Weave-GitOps?type=design&node-id=17897%3A199195&mode=design&t=fxC0g6SpQsA2Cyie-1

Note.PR.mov

Captura de pantalla 2023-09-21 a las 15 37 40

@AlinaGoaga
Copy link
Contributor Author

Thanks @mmoulian, will open a small PR separately to adjust the style :)

Comment on lines +94 to +95
{automation.type === "HelmRelease" &&
(automation as HelmRelease).kubeConfig === "" ? (

Choose a reason for hiding this comment

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

Seems that this change caused that only HelmRelease type can have detail now as reported in this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secrets not found errors on the applications page
4 participants