FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

setup-stack: fix error when RSPAMD_DMS_DKIM_D is not set by ap-wtioit · Pull Request #3827 · docker-mailserver/docker-mailserver · GitHub

/ docker-mailserver Public

setup-stack: fix error when RSPAMD_DMS_DKIM_D is not set - #3827

Merged
georglauterbach merged 3 commits into
docker-mailserver:masterfrom
ap-wtioit:master-fix_setup_stack
Jan 26, 2024
ConversationCommits (3)ChecksFiles changed
Merged

setup-stack: fix error when RSPAMD_DMS_DKIM_D is not set#3827
georglauterbach merged 3 commits into
docker-mailserver:masterfrom
ap-wtioit:master-fix_setup_stack

Conversation

ap-wtioit commented Jan 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

In our tests i got various:

chown: cannot access '': No such file or directory

and tracked it down to this line causing those messages when RSPAMD_DMS_DKIM_D had no value

Fixes #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

ap-wtioit force-pushed the master-fix_setup_stack branch from 439da31 to c7d79fd Compare January 25, 2024 10:10
setup-stack: fix error when RSPAMD_DMS_DKIM_D is not set
b36842c 
prevent messages like this
  chown: cannot access '': No such file or directory
when RSPAMD_DMS_DKIM_D has no value
ap-wtioit force-pushed the master-fix_setup_stack branch from c7d79fd to b36842c Compare January 25, 2024 10:14

polarathene commented Jan 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

This would be my fault I think with #3813

I assumed that the helper was imported by this point bringing that variable into scope?

docker-mailserver/target/scripts/helpers/rspamd.sh

Lines 21 to 22 in 22c6dae

readonly RSPAMD_DMS_D='/tmp/docker-mailserver/rspamd'
readonly RSPAMD_DMS_DKIM_D="${RSPAMD_DMS_D}/dkim"

docker-mailserver/target/scripts/start-mailserver.sh

Lines 10 to 11 in 22c6dae

# shellcheck source=./helpers/index.sh
source /usr/local/bin/helpers/index.sh

docker-mailserver/target/scripts/helpers/index.sh

Lines 4 to 7 in 22c6dae

# This file serves as a single import for all helpers
function _import_scripts() {
local PATH_TO_SCRIPTS='/usr/local/bin/helpers'

docker-mailserver/target/scripts/helpers/index.sh

Line 19 in 22c6dae

source "${PATH_TO_SCRIPTS}/rspamd.sh"

So a little curious why that is not the case 🤔

Ah, wrapped in a function that needs to be called to initialize:

docker-mailserver/target/scripts/helpers/rspamd.sh

Lines 13 to 17 in 22c6dae

# Calling this function brings common Rspamd-related environment variables
# into the current context. The environment variables are `readonly`, i.e.
# they cannot be modified. Use this function when you require common directory
# names, file names, etc.
function _rspamd_get_envs() {

polarathene added this to the v14.0.0 milestone Jan 25, 2024
georglauterbach self-assigned this Jan 25, 2024

georglauterbach commented Jan 25, 2024

Copy link
Copy Markdown
Member

I will need to look into this..

georglauterbach commented Jan 25, 2024

Copy link
Copy Markdown
Member

Indeed, _rspamd_get_envs seems to be required. I went over it and I thought the variables should are ins scope, but I guess scoping in Bash is all over the place. This seems to imply that the fix in v13.3.1 does not actually work?

georglauterbach requested changes Jan 25, 2024
View reviewed changes

georglauterbach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

CC @polarathene

Comment thread target/scripts/startup/setup-stack.sh Outdated
georglauterbach mentioned this pull request Jan 26, 2024
Merged
8 tasks
georglauterbach added the pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged label Jan 26, 2024
georglauterbach merged commit 9ac1102 into docker-mailserver:master Jan 26, 2024
polarathene mentioned this pull request Jan 29, 2024
Closed
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

polarathene polarathene left review comments
georglauterbach georglauterbach approved these changes

Assignees

ap-wtioit

georglauterbach

Labels

area/scripts kind/bug/fix A fix (PR) for a confirmed bug pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged service/security/dkim-dmarc-spf service/security/rspamd

Projects

None yet

Milestone

v14.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL