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

Specify VCL path and load multiple files at once with vcl.load in cli #3906

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion bin/varnishd/mgt/mgt.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ enum mcf_which_e {
void MCF_ParamConf(enum mcf_which_e, const char *param, const char *, ...)
v_printflike_(3, 4);

struct parspec *MCF_FindPar(const char *name);

void MCF_ParamSet(struct cli *, const char *param, const char *val);
void MCF_ParamProtect(struct cli *, const char *arg);
void MCF_DumpRstParam(void);
Expand Down Expand Up @@ -225,7 +227,7 @@ void STV_Init(void);
/* mgt_vcc.c */
void mgt_DumpBuiltin(void);
char *mgt_VccCompile(struct cli *, struct vclprog *, const char *vclname,
const char *vclsrc, const char *vclsrcfile, int C_flag);
const char *vclsrc, const char * const *vclsrcfiles, const char *vcl_path, int C_flag);

void mgt_vcl_init(void);
void mgt_vcl_startup(struct cli *, const char *vclsrc, const char *origin,
Expand Down
6 changes: 6 additions & 0 deletions bin/varnishd/mgt/mgt_param.c
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,12 @@ MCF_ParamProtect(struct cli *cli, const char *args)

/*--------------------------------------------------------------------*/

struct parspec *
MCF_FindPar(const char *name)
{
return mcf_findpar(name);
}

void
MCF_ParamSet(struct cli *cli, const char *param, const char *val)
{
Expand Down
23 changes: 17 additions & 6 deletions bin/varnishd/mgt/mgt_vcc.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ struct vcc_priv {
unsigned magic;
#define VCC_PRIV_MAGIC 0x70080cb8
const char *vclsrc;
const char *vclsrcfile;
const char * const *vclsrcfiles;
const char *vcl_path;
struct vsb *dir;
struct vsb *csrcfile;
struct vsb *libfile;
Expand Down Expand Up @@ -103,7 +104,7 @@ vcc_vext_iter_func(const char *filename, void *priv)
static void v_noreturn_ v_matchproto_(vsub_func_f)
run_vcc(void *priv)
{
struct vsb *sb = NULL;
struct vsb *sb = NULL, *vsb = NULL;
struct vclprog *vpg;
struct vcc_priv *vp;
struct vcc *vcc;
Expand All @@ -118,8 +119,17 @@ run_vcc(void *priv)
vcc = VCC_New();
AN(vcc);
VCC_Builtin_VCL(vcc, builtin_vcl);
VCC_VCL_path(vcc, mgt_vcl_path);
VCC_VMOD_path(vcc, mgt_vmod_path);
if (vp->vcl_path != NULL) {
vsb = VSB_new_auto();
AN(vsb);
VSB_printf(vsb, "%s:%s", vp->vcl_path, mgt_vcl_path);
AZ(VSB_finish(vsb));
VCC_VCL_path(vcc, VSB_data(vsb));
VSB_destroy(&vsb);
} else {
VCC_VCL_path(vcc, mgt_vcl_path);
}

#define VCC_FEATURE_BIT(U, l, d) \
VCC_Opt_ ## l(vcc, MGT_VCC_FEATURE(VCC_FEATURE_ ## U));
Expand All @@ -132,7 +142,7 @@ run_vcc(void *priv)
VTAILQ_FOREACH(vpg, &vclhead, list)
if (mcf_is_label(vpg))
VCC_Predef(vcc, "VCL_VCL", vpg->name);
i = VCC_Compile(vcc, &sb, vp->vclsrc, vp->vclsrcfile,
i = VCC_Compile(vcc, &sb, vp->vclsrc, vp->vclsrcfiles,
VGC_SRC, VGC_SYM);
if (VSB_len(sb))
printf("%s", VSB_data(sb));
Expand Down Expand Up @@ -347,7 +357,7 @@ mgt_vcc_fini_vp(struct vcc_priv *vp, int leave_lib)

char *
mgt_VccCompile(struct cli *cli, struct vclprog *vcl, const char *vclname,
const char *vclsrc, const char *vclsrcfile, int C_flag)
const char *vclsrc, const char * const *vclsrcfiles, const char *vcl_path, int C_flag)
{
struct vcc_priv vp[1];
struct vsb *sb;
Expand All @@ -361,7 +371,8 @@ mgt_VccCompile(struct cli *cli, struct vclprog *vcl, const char *vclname,

mgt_vcc_init_vp(vp);
vp->vclsrc = vclsrc;
vp->vclsrcfile = vclsrcfile;
vp->vclsrcfiles = vclsrcfiles;
vp->vcl_path = vcl_path;

/*
* The subdirectory must have a unique name to 100% certain evade
Expand Down
67 changes: 62 additions & 5 deletions bin/varnishd/mgt/mgt_vcl.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "mgt/mgt_vcl.h"
#include "common/heritage.h"

#include "mgt_param.h"
#include "vcli_serve.h"
#include "vct.h"
#include "vev.h"
Expand All @@ -56,6 +57,15 @@ struct vclstate {
static const struct vclstate VCL_STATE_ ## sym[1] = {{ str }};
#include "tbl/vcl_states.h"

#define FAIL_NO_ARG(a, p) \
do { \
if (a == NULL) { \
VCLI_Out(cli, p " requires an argument\n"); \
VCLI_SetResult(cli, CLIS_TOOFEW); \
return; \
} \
} while (0)

static const struct vclstate VCL_STATE_LABEL[1] = {{ "label" }};

static unsigned vcl_count;
Expand Down Expand Up @@ -428,7 +438,7 @@ mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, const struct vclstate *vs)

static struct vclprog *
mgt_new_vcl(struct cli *cli, const char *vclname, const char *vclsrc,
const char *vclsrcfile, const char *state, int C_flag)
const char * const *vclsrcfiles, const char *state, const char *vcl_path, int C_flag)
{
unsigned status;
char *lib, *p;
Expand All @@ -454,7 +464,7 @@ mgt_new_vcl(struct cli *cli, const char *vclname, const char *vclsrc,
return (NULL);

vp = mgt_vcl_add(vclname, vs);
lib = mgt_VccCompile(cli, vp, vclname, vclsrc, vclsrcfile, C_flag);
lib = mgt_VccCompile(cli, vp, vclname, vclsrc, vclsrcfiles, vcl_path, C_flag);
if (lib == NULL) {
mgt_vcl_del(vp);
return (NULL);
Expand Down Expand Up @@ -497,6 +507,7 @@ mgt_vcl_startup(struct cli *cli, const char *vclsrc, const char *vclname,
char buf[20];
static int n = 0;
struct vclprog *vp;
const char *vcls[] = {origin, NULL};

AZ(MCH_Running());

Expand All @@ -506,7 +517,7 @@ mgt_vcl_startup(struct cli *cli, const char *vclsrc, const char *vclname,
bprintf(buf, "boot%d", n++);
vclname = buf;
}
vp = mgt_new_vcl(cli, vclname, vclsrc, origin, NULL, C_flag);
vp = mgt_new_vcl(cli, vclname, vclsrc, vcls, NULL, NULL, C_flag);
if (vp != NULL) {
/* Last startup VCL becomes the automatically selected
* active VCL. */
Expand Down Expand Up @@ -580,12 +591,13 @@ mcf_vcl_inline(struct cli *cli, const char * const *av, void *priv)
{
struct vclprog *vp;

const char *vcls[] = {"<vcl.inline>", NULL};
(void)priv;

if (!mcf_find_no_vcl(cli, av[2]))
return;

vp = mgt_new_vcl(cli, av[2], av[3], "<vcl.inline>", av[4], 0);
vp = mgt_new_vcl(cli, av[2], av[3], vcls, av[4], NULL, 0);
if (vp != NULL && !MCH_Running())
VCLI_Out(cli, "VCL compiled.\n");
}
Expand All @@ -594,16 +606,60 @@ static void v_matchproto_(cli_func_t)
mcf_vcl_load(struct cli *cli, const char * const *av, void *priv)
{
struct vclprog *vp;
const char *vcls[] = {av[3], NULL};

(void)priv;
if (!mcf_find_no_vcl(cli, av[2]))
return;

vp = mgt_new_vcl(cli, av[2], NULL, av[3], av[4], 0);
vp = mgt_new_vcl(cli, av[2], NULL, vcls, av[4], NULL, 0);
if (vp != NULL && !MCH_Running())
VCLI_Out(cli, "VCL compiled.\n");
}

static void v_matchproto_(cli_func_t)
mcf_vcl_load_files(struct cli *cli, const char * const *av, void *priv)
{
struct vclprog *vp;
const struct parspec *pp;
int i = 2;
const char *state = NULL;
const char *vcl_path = NULL;

while (av[i] != NULL) {
if (!strcmp(av[i], "-s")) {
FAIL_NO_ARG(av[i+1], "-s");
state = av[i+1];
i += 2;
} else if (!strcmp(av[i], "-p")) {
FAIL_NO_ARG(av[i+1], "-p");
pp = MCF_FindPar("vcl_path");
if (pp->flags & PROTECTED) {
VCLI_SetResult(cli, CLIS_AUTH);
VCLI_Out(cli, "parameter \"vcl_path\" is protected.\n");
return;
}
Comment on lines +637 to +641
Copy link
Member

Choose a reason for hiding this comment

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

To clarify my previous comment, since both varnishd -r and vcl.load_files require admin privileges as per our security model, I would be fine relaxing this rule and allowing both a read-only global vcl_path and per-VCL additional path since they take precedence.

vcl_path = av[i+1];
i += 2;
} else {
break;
}
}

if (av[i] == NULL || av[i+1] == NULL) {
VCLI_SetResult(cli, CLIS_TOOFEW);
VCLI_Out(cli, "Too few arguments\n");
return;
}

(void)priv;
if (!mcf_find_no_vcl(cli, av[i]))
return;

vp = mgt_new_vcl(cli, av[i], NULL, &av[i+1], state, vcl_path, 0);
if (vp != NULL && !MCH_Running())
VCLI_Out(cli, "VCL compiled.\n");
}

static void v_matchproto_(cli_func_t)
mcf_vcl_state(struct cli *cli, const char * const *av, void *priv)
Expand Down Expand Up @@ -1093,6 +1149,7 @@ mgt_vcl_poker(const struct vev *e, int what)

static struct cli_proto cli_vcl[] = {
{ CLICMD_VCL_LOAD, "", mcf_vcl_load },
{ CLICMD_VCL_LOAD_FILES, "", mcf_vcl_load_files },
{ CLICMD_VCL_INLINE, "", mcf_vcl_inline },
{ CLICMD_VCL_USE, "", mcf_vcl_use },
{ CLICMD_VCL_STATE, "", mcf_vcl_state },
Expand Down
53 changes: 53 additions & 0 deletions bin/varnishtest/tests/c00123.vtc
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
varnishtest "Test vcl.load_files with multiple vcl files"

server s1 {
rxreq
txresp
} -start

varnish v1 -vcl+backend {} -start

shell {
(
echo 'vcl 4.0;'
echo 'backend default {'
echo ' .host="${s1_addr}";'
echo ' .port="${s1_port}";'
echo '}'
echo ''
echo 'sub vcl_deliver {'
echo ' set resp.http.test1 = "test1";'
echo '}'
) > ${tmpdir}/file1.vcl
}

shell {
(
echo 'vcl 4.0;'
echo 'sub vcl_deliver {'
echo ' set resp.http.test2 = "test2";'
echo '}'
) > ${tmpdir}/file2.vcl
}

shell {
(
echo 'vcl 4.0;'
echo 'sub vcl_deliver {'
echo ' set resp.http.test3 = "test3";'
echo '}'
) > ${tmpdir}/file3.vcl
}

varnish v1 -cliok {vcl.load_files -s cold foo ${tmpdir}/file1.vcl ${tmpdir}/file2.vcl ${tmpdir}/file3.vcl}
varnish v1 -cliexpect "VCL 'foo' is cold" {vcl.use foo}
varnish v1 -cliok {vcl.state foo warm}
varnish v1 -cliok {vcl.use foo}

client c1 {
txreq
rxresp
expect resp.http.test1 == "test1"
expect resp.http.test2 == "test2"
expect resp.http.test3 == "test3"
} -run
40 changes: 40 additions & 0 deletions bin/varnishtest/tests/c00124.vtc
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
varnishtest "Test vcl.load_files -p arg"

server s1 {
rxreq
txresp -status 202
} -start

varnish v1 -vcl+backend {} -start

shell {
(
echo 'vcl 4.0;'
echo 'backend default {'
echo ' .host="${s1_addr}";'
echo ' .port="${s1_port}";'
echo '}'
echo ''
echo 'include "inc.vcl";'
) > ${tmpdir}/c00123.vcl
}

shell {
(
echo 'sub vcl_deliver {'
echo ' set resp.http.test = "c00123";'
echo '}'
) > ${tmpdir}/inc.vcl
}

varnish v1 -cliexpect "No such file or directory" {vcl.load_files foo c00123.vcl}

varnish v1 -cliok {vcl.load_files -p ${tmpdir} foo c00123.vcl}
varnish v1 -cliok {vcl.use foo}

client c1 {
txreq
rxresp
expect resp.http.test == "c00123"
expect resp.status == 202
} -run
13 changes: 13 additions & 0 deletions doc/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ Varnish Cache NEXT (2023-09-15)
.. _3908: https://github.com/varnishcache/varnish-cache/pull/3908
.. _3911: https://github.com/varnishcache/varnish-cache/issues/3911

* A new ``vcl.load_files`` command has been added to cli. It has
the following syntax:

``vcl.load_file [-s auto|cold|warm] [-p vcl_path]
<configname> <filename1> [filename2, ...]``

It is similar to ``vcl.load`` command except that it turns the state argument
into ``[-s auto|cold|warm]``, it has an optional ``-p`` argument that is prepended
to ``vcl_path`` parameter when loading the vcls, and it allows to specify multiple
vcl files to load, they are parsed in the order they are provided, and treated
as if they were concatenated and merged into a single vcl file. Note that this command
will fail when ``-p`` is specified and ``vcl_path`` is protected (read only).

================================
Varnish Cache 7.3.0 (2023-03-15)
================================
Expand Down
2 changes: 1 addition & 1 deletion include/libvcc.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ void VCC_VEXT(struct vcc *, const char *);
#include "tbl/vcc_feature_bits.h"

int VCC_Compile(struct vcc *, struct vsb **,
const char *, const char *, const char *, const char *);
const char *, const char *const *, const char *, const char *);
13 changes: 13 additions & 0 deletions include/tbl/cli_cmds.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,19 @@ CLI_CMD(VCL_LOAD,
2, 3
)

CLI_CMD(VCL_LOAD_FILES,
"vcl.load_files",
"vcl.load_files [-s auto|cold|warm] [-p vcl_path] <configname> <filename1> [filename2, ...]",
"Compile and load multiple VCL files under the name provided.",
" When multiple files are specified, they are parsed in the order"
" they were provided and treated as if they were concatenated and merged into"
" a single vcl file."
" The optional ``-p`` argument specifies the path of the vcl files (and included files),"
" it is prepended to the ``vcl_path`` parameter. This command will fail when ``-p`` is specified"
" and ``vcl_path`` is protected (read only).",
2, -1
)

CLI_CMD(VCL_INLINE,
"vcl.inline",
"vcl.inline <configname> <quoted_VCLstring> [auto|cold|warm]",
Expand Down
Loading