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

VCL include +glob with relative path is not working #4249

Open
masarlabs opened this issue Jan 6, 2025 · 4 comments · May be fixed by #4250
Open

VCL include +glob with relative path is not working #4249

masarlabs opened this issue Jan 6, 2025 · 4 comments · May be fixed by #4250
Assignees
Labels
a=Implement Issue is ready for implementation b=bug c=varnishd

Comments

@masarlabs
Copy link

masarlabs commented Jan 6, 2025

Expected Behavior

I created a minimal default.vcl that includes an empty file

vcl 4.1;

backend default {
  .host = "[127.0.0.1]:3081";
}

include       "test.d/test.vcl";
include +glob "/etc/varnish/test.d/*.vcl";
include +glob "test.d/*.vcl";

then test the code with

varnishd -C -f /etc/varnish/default.vcl

it should go without error

Current Behavior

the first two includes work as expected, but the last one says

Message from VCC-compiler:
glob pattern matched no files.
('/etc/varnish/default.vcl' Line 12 Pos 15)
include +glob "test.d/*.vcl";
--------------##############-
Running VCC-compiler failed, exited with 2
VCL compilation failed

Possible Solution

I think that current directory is not set correctly because it works with full path.

Steps to Reproduce (for bugs)

No response

Context

Cannot use relative path in vcl

Varnish Cache version

varnishd (varnish-7.6.1 revision c3d5882)

Operating system

alpine 3.21.0

Source of binary packages used (if any)

No response

@nigoroll
Copy link
Member

nigoroll commented Jan 6, 2025

bugwash agrees that with +glob, relative paths should be tried relative to vcl_path members.
@masarlabs I am going to edit your follow-up comment into the first note and remove the former.

@nigoroll nigoroll added b=bug c=varnishd a=Implement Issue is ready for implementation labels Jan 6, 2025
@varnishcache varnishcache deleted a comment from masarlabs Jan 6, 2025
@walid-git walid-git self-assigned this Jan 7, 2025
walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
@walid-git walid-git linked a pull request Jan 7, 2025 that will close this issue
@walid-git
Copy link
Member

@masarlabs can you give #4250 a try ?

walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
We preivously called glob() on the included file name regardless of whether it
was an absolute or relative path. As a result, relative paths were not appended
to vcl_path elements before checking if the glob expression matched anything.

It is not possible to (re)use VFIL_searchpath here, since VFIL_searchpath uses
access() to check that the file exists, and that would fail in this case.

Fixes: varnishcache#4249
walid-git added a commit to walid-git/varnish-cache that referenced this issue Jan 7, 2025
@masarlabs
Copy link
Author

Compiled

git clone https://github.com/walid-git/varnish-cache.git
cd varnish-cache/
git switch origin/fix_4249
sh autogen.sh
sh configure
make
make install

Version

# varnishd -V
varnishd (varnish-trunk revision 0edd8bcfda20ae5fbf2dd360ea809a687ef07379)
Copyright (c) 2006 Verdens Gang AS
Copyright (c) 2006-2024 Varnish Software
Copyright 2010-2024 UPLEX - Nils Goroll Systemoptimierung

Test

# varnishd -C -p vcl_path=/etc/varnish -f /etc/varnish/default.vcl
...

No error, it seems to work.
Thanks

@walid-git
Copy link
Member

No error, it seems to work.
Thanks

Thanks for confirming. I will keep the issue open until a fix (#4250 or other) is merged.

@walid-git walid-git reopened this Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a=Implement Issue is ready for implementation b=bug c=varnishd
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants