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

Duplicated application groups #85

Open
angelnu opened this issue Oct 23, 2022 · 4 comments
Open

Duplicated application groups #85

angelnu opened this issue Oct 23, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@angelnu
Copy link

angelnu commented Oct 23, 2022

Describe the bug
Application groups injected via ingress annotations and customApps are not merged. This results in the group appearing twice:

image

I checked the that they written exactly the same: Admin

To Reproduce
Steps to reproduce the behavior:

  1. Add following annotation
        hajimari.io/enable: "true"
        hajimari.io/icon: logos:kubernetes
        hajimari.io/info: Kubernetes dashboard
        hajimari.io/group: Admin
    
  2. Add following custom app
            customApps:
            - group: Admin
              apps:
                - name: Test
                  url: 'https://example.com'
                  icon: 'mdi:test-tube'
                  info: This is a test app
    
  3. See the duplication

Expected behavior

Both apps showing in the same group

Screenshots

See above

Desktop (please complete the following information):

  • OS: Windows
  • Browser Frirefox
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version 105.0.3

Additional context
NA

@ullbergm
Copy link
Contributor

I saw this when the case was different in the annotation and the custom app, not sure if there are other cases where it happens as well.

@angelnu
Copy link
Author

angelnu commented Oct 30, 2022

@ullbergm - that was my thought as I had problems with the case before. But I double-checked including seeing if there were any extra leading/trailing spaces and there are none I can see.

Did the de-duplication work for you when the cases match between the ingress and custom app?

@ullbergm
Copy link
Contributor

ullbergm commented Oct 30, 2022

It did.

I think the problem is that when customApps are loaded from the config the group name is turned in to lower-case but that is not happening with ingressApps:
https://github.com/toboshii/hajimari/blob/main/internal/hajimari/customapps/customapps.go#L30
vs
https://github.com/toboshii/hajimari/blob/main/internal/kube/wrappers/ingress.go#L61

@toboshii The current fix I put in is to normalize the group names when loaded from the config/kubernetes api. The drawback is that if the theme changes in the future then we may want to retain the case of the group names.

Incidentally, there is also an issue if you have extra spaces like @angelnu said which is fixed with a strings.TrimSpace() when loading/comparing the objects.

edit: changed my mind

@angelnu
Copy link
Author

angelnu commented Oct 30, 2022

Thanks @ullbergm - I can confirm that after removing the capital case (Admin -> admin) then both groups are merged as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants