Skip to content

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

Merged
georglauterbach merged 3 commits intodocker-mailserver:masterfrom
ap-wtioit:master-fix_setup_stack
Jan 26, 2024
Merged

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

Conversation

@ap-wtioit
Copy link
Copy Markdown
Contributor

@ap-wtioit ap-wtioit commented Jan 25, 2024 •
edited
Loading

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 ap-wtioit force-pushed the master-fix_setup_stack branch from 439da31 to c7d79fd Compare January 25, 2024 10:10
@ap-wtioit
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 ap-wtioit force-pushed the master-fix_setup_stack branch from c7d79fd to b36842c Compare January 25, 2024 10:14
@polarathene
Copy link
Copy Markdown
Member

polarathene commented Jan 25, 2024 •
edited
Loading

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 polarathene added this to the v14.0.0 milestone Jan 25, 2024
@georglauterbach georglauterbach self-assigned this Jan 25, 2024
@georglauterbach
Copy link
Copy Markdown
Member

georglauterbach commented Jan 25, 2024

I will need to look into this..

@georglauterbach
Copy link
Copy Markdown
Member

georglauterbach commented Jan 25, 2024

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
georglauterbach requested changes Jan 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

CC @polarathene

Comment thread target/scripts/startup/setup-stack.sh Outdated
@georglauterbach georglauterbach mentioned this pull request Jan 26, 2024
Merged
8 tasks
@georglauterbach 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 georglauterbach merged commit 9ac1102 into docker-mailserver:master Jan 26, 2024
@polarathene polarathene mentioned this pull request Jan 29, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@polarathene polarathene polarathene left review comments

@georglauterbach georglauterbach georglauterbach approved these changes

Assignees

@ap-wtioit ap-wtioit

@georglauterbach 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

@ap-wtioit @polarathene @georglauterbach