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

Notifications service doesn't like Kubernetes ConfigMaps #6567

Closed
wkloucek opened this issue Jun 20, 2023 · 5 comments · Fixed by #6574
Closed

Notifications service doesn't like Kubernetes ConfigMaps #6567

wkloucek opened this issue Jun 20, 2023 · 5 comments · Fixed by #6574
Assignees
Labels

Comments

@wkloucek
Copy link
Contributor

White working on owncloud/ocis-charts#330, I ran into this:

kubectl -n ocis logs -f deployment/notifications
{"level":"error","service":"notifications","error":"read /etc/ocis/notifications/templates/html/img/..data: is a directory","event":"ShareCreated","time":"2023-06-20T15:09:57.6929667Z","line":"github.com/owncloud/ocis/v2/services/notifications/pkg/service/shares.go:59","message":"could not get render the email"}

Looks like the current implementation doesn't like Kubernetes ConfigMaps, because the template path looks like this when mounting these files as configmap:

kubectl exec -it -n ocis pods/notifications-76bbd45d77-znbf2 -- sh 
~ $ cd /etc/ocis/notifications/
/etc/ocis/notifications $ tree -a
.
└── templates
    ├── html
    │   ├── ..2023_06_20_15_09_09.176225182
    │   │   └── email.html.tmpl
    │   ├── ..data -> ..2023_06_20_15_09_09.176225182
    │   ├── email.html.tmpl -> ..data/email.html.tmpl
    │   └── img
    │       ├── ..2023_06_20_15_09_09.1371932421
    │       │   ├── logo-mail.gif
    │       │   ├── logo-mail.jpeg
    │       │   └── logo-mail.png
    │       ├── ..data -> ..2023_06_20_15_09_09.1371932421
    │       ├── logo-mail.gif -> ..data/logo-mail.gif
    │       ├── logo-mail.jpeg -> ..data/logo-mail.jpeg
    │       └── logo-mail.png -> ..data/logo-mail.png
    └── text
        ├── ..2023_06_20_15_09_09.4107791090
        │   └── email.text.tmpl
        ├── ..data -> ..2023_06_20_15_09_09.4107791090
        └── email.text.tmpl -> ..data/email.text.tmpl

10 directories, 10 files

Originally posted by @wkloucek in owncloud/ocis-charts#330 (comment)

@2403905 2403905 self-assigned this Jun 20, 2023
@2403905
Copy link
Contributor

2403905 commented Jun 20, 2023

@micbar @kobergj @mmattel Now we have the default path like {NOTIFICATIONS_EMAIL_TEMPLATE_PATH}/templates/...
I can change the path to {NOTIFICATIONS_EMAIL_TEMPLATE_PATH}/email/templates/... Is it ok to change the path and readme on this release step?

@mmattel
Copy link
Contributor

mmattel commented Jun 20, 2023

@2403905 Changing the path is totally fine. Just change it to what is necessary but also do a readme change. I will then take care to to the changes in the admin docs.

I also have created an issue in docs-ocis which covers notable (upgrade relevant) and breaking changes. When this goes forward with a PR, we should add that to the list there.

@2403905
Copy link
Contributor

2403905 commented Jun 21, 2023

look like I misunderstood, It doesn't matter which path will be under the NOTIFICATIONS_EMAIL_TEMPLATE_PATH, the Kubernetes ConfigMaps anyway add the extra. @wkloucek Is it correct?

@wkloucek
Copy link
Contributor Author

look like I misunderstood, It doesn't matter which path will be under the NOTIFICATIONS_EMAIL_TEMPLATE_PATH, the Kubernetes ConfigMaps anyway add the extra. @wkloucek Is it correct?

My understanding is that the problem is that

func readImages(emailTemplatePath string) (map[string][]byte, error) {
dir := filepath.Join(emailTemplatePath, imgDir)
entries, err := os.ReadDir(dir)
if err != nil {
return nil, err
}
return read(entries, os.DirFS(emailTemplatePath))
}
func read(entries []fs.DirEntry, fsys fs.FS) (map[string][]byte, error) {
list := make(map[string][]byte)
for _, e := range entries {
if !e.IsDir() {
file, err := fs.ReadFile(fsys, filepath.Join(imgDir, e.Name()))
if err != nil {
return nil, err
}
if !validateMime(file) {
continue
}
list[e.Name()] = file
}
}
return list, nil
}
tries to do something with a directory, even it has some logic, that should skip directories:

@wkloucek
Copy link
Contributor Author

@2403905 fix confirmed in Kubernetes 👍

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

Successfully merging a pull request may close this issue.

3 participants