Skip to content

Commit

Permalink
call validate_resource also from configure add.
Browse files Browse the repository at this point in the history 
This is required to check if all required items are set on job resources,
because job resources have special treatment during reading the
configuration, as they can inherit values from jobdefs.
  • Loading branch information
@joergsteffens
joergsteffens committed Aug 25, 2016
1 parent bcbb17f commit d5c4ba4
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 86 deletions.
184 changes: 102 additions & 82 deletions src/dird/dird_conf.c
View file
Expand Up @@ -1256,17 +1256,29 @@ static void propagate_resource(RES_ITEM *items, BRSRES *source, BRSRES *dest)
}
}


/*
* Ensure that all required items are present
*/
static bool validate_resource(RES_ITEM *items, BRSRES *res, const char *res_type)
bool validate_resource(int res_type, RES_ITEM *items, BRSRES *res)
{
if (res_type == R_JOBDEFS) {
/*
* a jobdef don't have to be fully defined.
*/
return true;
} else if (res_type == R_JOB) {
if (!((JOBRES *)res)->validate()) {
return false;
}
}

for (int i = 0; items[i].name; i++) {
if (items[i].flags & CFG_ITEM_REQUIRED) {
if (!bit_is_set(i, res->hdr.item_present)) {
Jmsg(NULL, M_ERROR, 0,
_("\"%s\" directive in %s \"%s\" resource is required, but not found.\n"),
items[i].name, res_type, res->name());
items[i].name, res_to_str(res_type), res->name());
return false;
}
}
Expand All @@ -1275,9 +1287,47 @@ static bool validate_resource(RES_ITEM *items, BRSRES *res, const char *res_type
* If this triggers, take a look at lib/parse_conf.h
*/
if (i >= MAX_RES_ITEMS) {
Emsg1(M_ERROR, 0, _("Too many items in %s resource\n"), res_type);
Emsg1(M_ERROR, 0, _("Too many items in %s resource\n"), res_to_str(res_type));
return false;
}
}

return true;
}

bool JOBRES::validate()
{
/*
* For Copy and Migrate we can have Jobs without a client or fileset.
* As for a copy we use the original Job as a reference for the Read storage
* we also don't need to check if there is an explicit storage definition in
* either the Job or the Read pool.
*/
switch (JobType) {
case JT_COPY:
case JT_MIGRATE:
break;
default:
/*
* All others must have a client and fileset.
*/
if (!client) {
Jmsg(NULL, M_ERROR, 0,
_("\"client\" directive in Job \"%s\" resource is required, but not found.\n"), name());
return false;
}

if (!fileset) {
Jmsg(NULL, M_ERROR, 0,
_("\"fileset\" directive in Job \"%s\" resource is required, but not found.\n"), name());
return false;
}

if (!storage && !pool->storage) {
Jmsg(NULL, M_ERROR, 0, _("No storage specified in Job \"%s\" nor in Pool.\n"), name());
return false;
}
break;
}

return true;
Expand Down Expand Up @@ -2958,7 +3008,7 @@ bool save_resource(int type, RES_ITEM *items, int pass)
/*
* Ensure that all required items are present
*/
if (!validate_resource(items, &res_all.res_dir, resources[rindex].name)) {
if (!validate_resource(type, items, &res_all.res_dir)) {
return false;
}
}
Expand Down Expand Up @@ -3008,6 +3058,50 @@ bool save_resource(int type, RES_ITEM *items, int pass)
return true;
}

bool propagate_jobdefs(int res_type, JOBRES *res)
{
JOBRES *jobdefs = NULL;

if (!res->jobdefs) {
return true;
}

/*
* Don't allow the JobDefs pointing to itself.
*/
if (res->jobdefs == res) {
return false;
}

if (res_type == R_JOB) {
jobdefs = res->jobdefs;

/*
* Handle RunScripts alists specifically
*/
if (jobdefs->RunScripts) {
RUNSCRIPT *rs, *elt;

if (!res->RunScripts) {
res->RunScripts = New(alist(10, not_owned_by_alist));
}

foreach_alist(rs, jobdefs->RunScripts) {
elt = copy_runscript(rs);
elt->from_jobdef = true;
res->RunScripts->append(elt); /* we have to free it */
}
}
}

/*
* Transfer default items from JobDefs Resource
*/
propagate_resource(job_items, res->jobdefs, res);

return true;
}

/*
* Populate Job Defaults (e.g. JobDefs)
*/
Expand All @@ -3020,89 +3114,23 @@ static inline bool populate_jobdefs()
* Propagate the content of a JobDefs to another.
*/
foreach_res(jobdefs, R_JOBDEFS) {
/*
* Don't allow the JobDefs pointing to itself.
*/
if (!jobdefs->jobdefs || jobdefs->jobdefs == jobdefs) {
continue;
}

propagate_resource(job_items, jobdefs->jobdefs, jobdefs);
propagate_jobdefs(R_JOBDEFS, jobdefs);
}

/*
* Propagate the content of the JobDefs to the actual Job.
*/
foreach_res(job, R_JOB) {
if (job->jobdefs) {
jobdefs = job->jobdefs;

/*
* Handle RunScripts alists specifically
*/
if (jobdefs->RunScripts) {
RUNSCRIPT *rs, *elt;

if (!job->RunScripts) {
job->RunScripts = New(alist(10, not_owned_by_alist));
}

foreach_alist(rs, jobdefs->RunScripts) {
elt = copy_runscript(rs);
elt->from_jobdef = true;
job->RunScripts->append(elt); /* we have to free it */
}
}

/*
* Transfer default items from JobDefs Resource
*/
propagate_resource(job_items, jobdefs, job);
}
propagate_jobdefs(R_JOB, job);

/*
* Ensure that all required items are present
*/
if (!validate_resource(job_items, job, "Job")) {
if (!validate_resource(R_JOB, job_items, job)) {
retval = false;
goto bail_out;
}

/*
* For Copy and Migrate we can have Jobs without a client or fileset.
* As for a copy we use the original Job as a reference for the Read storage
* we also don't need to check if there is an explicit storage definition in
* either the Job or the Read pool.
*/
switch (job->JobType) {
case JT_COPY:
case JT_MIGRATE:
break;
default:
/*
* All others must have a client and fileset.
*/
if (!job->client) {
Jmsg(NULL, M_ERROR_TERM, 0,
_("\"client\" directive in Job \"%s\" resource is required, but not found.\n"), job->name());
retval = false;
goto bail_out;
}

if (!job->fileset) {
Jmsg(NULL, M_ERROR_TERM, 0,
_("\"fileset\" directive in Job \"%s\" resource is required, but not found.\n"), job->name());
retval = false;
goto bail_out;
}

if (!job->storage && !job->pool->storage) {
Jmsg(NULL, M_FATAL, 0, _("No storage specified in Job \"%s\" nor in Pool.\n"), job->name());
retval = false;
goto bail_out;
}
break;
}
} /* End loop over Job res */

bail_out:
Expand All @@ -3111,15 +3139,7 @@ static inline bool populate_jobdefs()

bool populate_defs()
{
bool retval;

retval = populate_jobdefs();
if (!retval) {
goto bail_out;
}

bail_out:
return retval;
return populate_jobdefs();
}

static void store_pooltype(LEX *lc, RES_ITEM *item, int index, int pass)
Expand Down
3 changes: 3 additions & 0 deletions src/dird/dird_conf.h
View file
Expand Up @@ -443,6 +443,7 @@ class JOBRES : public BRSRES {
runtime_job_status_t *rjs; /* Runtime Job Status */

/* Methods */
bool validate();
};

#undef MAX_FOPTS
Expand Down Expand Up @@ -621,6 +622,8 @@ union URES {
};

void init_dir_config(CONFIG *config, const char *configfile, int exit_code);
bool propagate_jobdefs(int res_type, JOBRES *res);
bool validate_resource(int type, RES_ITEM *items, BRSRES *res);

#define GetPoolResWithName(x) ((POOLRES *)GetResWithName(R_POOL, (x)))
#define GetStoreResWithName(x) ((STORERES *)GetResWithName(R_STORAGE, (x)))
Expand Down
20 changes: 20 additions & 0 deletions src/dird/ua_configure.c
View file
Expand Up @@ -288,6 +288,7 @@ static inline bool configure_add_resource(UAContext *ua, int first_parameter, RE
POOL_MEM filename_tmp(PM_FNAME);
POOL_MEM filename(PM_FNAME);
POOL_MEM temp(PM_FNAME);
JOBRES *res = NULL;

if (!configure_create_resource_string(ua, first_parameter, res_table, name, resource)) {
return false;
Expand Down Expand Up @@ -316,6 +317,24 @@ static inline bool configure_add_resource(UAContext *ua, int first_parameter, RE
return false;
}

/*
* parse_config_file has already done some validation.
* However, it skipped at least some checks for R_JOB
* (reason: a job can get values from jobdefs,
* and the value propagation happens after reading the full configuration)
* therefore we explicitly check the new resource here.
*/
if ((res_table->rcode == R_JOB) || (res_table->rcode == R_JOBDEFS)) {
res = (JOBRES *)GetResWithName(res_table->rcode, name.c_str());
propagate_jobdefs(res_table->rcode, res);
if (!validate_resource(res_table->rcode, res_table->items, (BRSRES *)res)) {
ua->error_msg("failed to create config resource \"%s\"\n", name.c_str());
unlink(filename_tmp.c_str());
my_config->remove_resource(res_table->rcode, name.c_str());
return false;
}
}

/*
* new config resource is working fine. Rename file to its permanent name.
*/
Expand Down Expand Up @@ -425,3 +444,4 @@ bool configure_cmd(UAContext *ua, const char *cmd)

return result;
}

31 changes: 29 additions & 2 deletions src/lib/parse_conf.c
View file
Expand Up @@ -352,14 +352,41 @@ bool CONFIG::parse_config_file(const char *cf, void *caller_ctx, LEX_ERROR_HANDL
return false;
}

const char *CONFIG::get_resource_type_name(int code)
{
return res_to_str(code);
}

int CONFIG::get_resource_table_index(int resource_type)
{
int rindex = -1;

if ((resource_type >= m_r_first) && (resource_type <= m_r_last)) {
rindex = resource_type = m_r_first;
}

return rindex;
}

RES_TABLE *CONFIG::get_resource_table(int resource_type)
{
RES_TABLE *result = NULL;
int rindex = get_resource_table_index(resource_type);

if (rindex >= 0) {
result = &m_resources[rindex];
}

return result;
}

RES_TABLE *CONFIG::get_resource_table(const char *resource_type)
RES_TABLE *CONFIG::get_resource_table(const char *resource_type_name)
{
RES_TABLE *result = NULL;
int i;

for (i = 0; m_resources[i].name; i++) {
if (bstrcasecmp(m_resources[i].name, resource_type)) {
if (bstrcasecmp(m_resources[i].name, resource_type_name)) {
result = &m_resources[i];
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/lib/parse_conf.h
View file
Expand Up @@ -329,7 +329,7 @@ class BRSRES {

/* Methods */
char *name() const;
bool print_config(POOL_MEM &buf, bool hide_sensitive_data = false);
bool print_config(POOL_MEM &buf, bool hide_sensitive_data = false, bool verbose = false);
/*
* validate can be defined by inherited classes,
* when special rules for this resource type must be checked.
Expand All @@ -338,6 +338,7 @@ class BRSRES {
};

inline char *BRSRES::name() const { return this->hdr.name; }
inline bool BRSRES::validate() { return true; }

/*
* Message Resource
Expand Down Expand Up @@ -435,7 +436,10 @@ class CONFIG {
bool remove_resource(int type, const char *name);
void dump_resources(void sendit(void *sock, const char *fmt, ...),
void *sock, bool hide_sensitive_data = false);
RES_TABLE *get_resource_table(const char *resource_type);
const char *get_resource_type_name(int code);
int get_resource_code(const char *resource_type);
RES_TABLE *get_resource_table(int resource_type);
RES_TABLE *get_resource_table(const char *resource_type_name);
int get_resource_item_index(RES_ITEM *res_table, const char *item);
RES_ITEM *get_resource_item(RES_ITEM *res_table, const char *item);
bool get_path_of_resource(POOL_MEM &path, const char *component, const char *resourcetype,
Expand All @@ -457,6 +461,7 @@ class CONFIG {
bool get_config_file(POOL_MEM &full_path, const char *config_dir, const char *config_filename);
bool get_config_include_path(POOL_MEM &full_path, const char *config_dir);
bool find_config_path(POOL_MEM &full_path);
int get_resource_table_index(int resource_type);
};

CONFIG *new_config_parser();
Expand Down

0 comments on commit d5c4ba4

Please sign in to comment.