Skip to content

Commit 63b8fea

Browse files
larsewilarsewi
authored
Merge pull request #5894 from nickanderson/CFE-4591/master
CFE-4591: move_obstructions should replace symlinks
2 parents ef005c5 + 4f3bb4a commit 63b8fea

File tree

4 files changed

+273
-0
lines changed

4 files changed

+273
-0
lines changed

‎cf-agent/verify_files.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,20 @@ static PromiseResult VerifyFilePromise(EvalContext *ctx, char *path, const Promi
587587
result = PromiseResultUpdate(result, ScheduleLinkOperation(ctx, path, a.link.source, &a, pp));
588588
}
589589

590+
if (a.haveedit || a.content || a.edit_template_string)
591+
{
592+
if (exists || link)
593+
{
594+
if (!HandleFileObstruction(ctx, changes_path, &oslb, &a, pp, &result))
595+
{
596+
goto exit;
597+
}
598+
// After moving, it no longer exists at the original path
599+
exists = (lstat(changes_path, &oslb) != -1);
600+
link = false;
601+
}
602+
}
603+
590604
/* Phase 3a - direct content */
591605

592606
if (a.content)

‎cf-agent/verify_files_utils.c‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
included file COSL.txt.
2323
*/
2424

25+
#include <stddef.h>
2526
#include <sys/types.h>
2627
#include <verify_files_utils.h>
2728

@@ -2775,6 +2776,57 @@ static PromiseResult VerifyFileAttributes(EvalContext *ctx, const char *file, co
27752776
return result;
27762777
}
27772778

2779+
bool HandleFileObstruction(EvalContext *ctx, const char *path, const struct stat *sb, const Attributes *attr, const Promise *pp, PromiseResult *result)
2780+
{
2781+
// Tell static analysis tools that these pointers do not need to be checked for NULL before dereferencing
2782+
assert(sb != NULL);
2783+
assert(attr != NULL);
2784+
2785+
const mode_t st_mode = sb->st_mode;
2786+
const bool move_obstructions = attr->move_obstructions;
2787+
2788+
// If path exists, but is not a regular file, it's an obstruction
2789+
if (!S_ISREG(st_mode))
2790+
{
2791+
if (move_obstructions)
2792+
{
2793+
if (MakingChanges(ctx, pp, attr, result, "Moving obstructing file '%s'", path))
2794+
{
2795+
char backup[CF_BUFSIZE];
2796+
int ret = snprintf(backup, sizeof(backup), "%s.cf-moved", path);
2797+
if (ret < 0 || (size_t) ret >= sizeof(backup))
2798+
{
2799+
RecordFailure(ctx, pp, attr, "Could not move obstruction '%s': Path too long",
2800+
path);
2801+
*result = PromiseResultUpdate(*result, PROMISE_RESULT_FAIL);
2802+
return false;
2803+
}
2804+
2805+
if (rename(path, backup) == -1)
2806+
{
2807+
RecordFailure(ctx, pp, attr, "Could not move obstruction '%s' to '%s'. (rename: %s)",
2808+
path, backup, GetErrorStr());
2809+
*result = PromiseResultUpdate(*result, PROMISE_RESULT_FAIL);
2810+
return false;
2811+
}
2812+
else
2813+
{
2814+
RecordChange(ctx, pp, attr, "Moved obstructing path '%s' to '%s'", path, backup);
2815+
*result = PromiseResultUpdate(*result, PROMISE_RESULT_CHANGE);
2816+
return true;
2817+
}
2818+
}
2819+
}
2820+
else if (!S_ISLNK(st_mode))
2821+
{
2822+
RecordFailure(ctx, pp, attr, "Path '%s' is not a regular file and move_obstructions is not set", path);
2823+
*result = PromiseResultUpdate(*result, PROMISE_RESULT_FAIL);
2824+
return false;
2825+
}
2826+
}
2827+
return true;
2828+
}
2829+
27782830
bool DepthSearch(EvalContext *ctx, char *name, const struct stat *sb, int rlevel, const Attributes *attr,
27792831
const Promise *pp, dev_t rootdevice, PromiseResult *result)
27802832
{

‎cf-agent/verify_files_utils.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ extern StringSet *SINGLE_COPY_CACHE;
3535
void SetFileAutoDefineList(const Rlist *auto_define_list);
3636

3737
void VerifyFileLeaf(EvalContext *ctx, char *path, const struct stat *sb, const Attributes *attr, const Promise *pp, PromiseResult *result);
38+
bool HandleFileObstruction(EvalContext *ctx, const char *path, const struct stat *sb, const Attributes *attr, const Promise *pp, PromiseResult *result);
3839
bool DepthSearch(EvalContext *ctx, char *name, const struct stat *sb, int rlevel, const Attributes *attr, const Promise *pp, dev_t rootdevice, PromiseResult *result);
3940
bool CfCreateFile(EvalContext *ctx, char *file, const Promise *pp, const Attributes *attr, PromiseResult *result_out);
4041
void SetSearchDevice(struct stat *sb, const Promise *pp);

‎tests/acceptance/10_files/move_obstructions-promiser-is-symlink.cf‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
2+
##############################################################################
3+
#
4+
# Test that move_obstructions works consistently for various types of files
5+
# promises targeting a specific file where the promiser is a symlink
6+
# - copy_from
7+
#
8+
##############################################################################
9+
10+
body file control
11+
{
12+
inputs => { "../default.cf.sub" };
13+
}
14+
15+
##############################################################################
16+
bundle agent __main__
17+
{
18+
methods:
19+
"init";
20+
"test";
21+
"check";
22+
"cleanup";
23+
}
24+
25+
bundle agent cleanup
26+
{
27+
vars:
28+
"potential_files_to_delete"
29+
slist => {
30+
# The files we tested
31+
"@(init.test_files)",
32+
"$(init.orig_ln_target_filename)",
33+
# .cfsaved files get created as copy_from replaces files
34+
maplist( "$(this).cfsaved", @(init.test_files) ),
35+
# The special case source file to copy from
36+
maplist( "$(this).source", @(init.test_files) ),
37+
};
38+
39+
files:
40+
"$(potential_files_to_delete)"
41+
delete => tidy,
42+
if => fileexists( "$(this.promiser)" );
43+
}
44+
45+
bundle agent init
46+
{
47+
vars:
48+
# Create test files that will serve as obstructions
49+
"test_files" slist => {
50+
"$(G.testdir)/copy_from",
51+
"$(G.testdir)/content",
52+
"$(G.testdir)/edit_template_string_inline_mustache",
53+
"$(G.testdir)/edit_template_cfengine",
54+
"$(G.testdir)/edit_template_mustache",
55+
#"$(G.testdir)/edit_line",
56+
};
57+
58+
"orig_ln_target_filename"
59+
string => "$(G.testdir)/orig_ln_target";
60+
61+
"orig_ln_target_content"
62+
string => "This content originally lived in a symlink target";
63+
64+
files:
65+
# Here we initialize a file which will be a symlinks target
66+
"$(orig_ln_target_filename)"
67+
create => "true",
68+
content => "$(orig_ln_target_content)";
69+
70+
# Here we have a symlink that we will be targeting with a content files promise
71+
"$(test_files)"
72+
create => "true",
73+
move_obstructions => "true",
74+
link_from => ln_s( "$(G.testdir)/orig_ln_target" );
75+
76+
reports:
77+
DEBUG::
78+
"$(test_files) initalized"
79+
if => and( fileexists( "$(test_files)" ),
80+
islink( $(test_files) ) );
81+
}
82+
83+
body link_from ln_s(x)
84+
{
85+
link_type => "symlink";
86+
source => "$(x)";
87+
when_no_source => "force";
88+
}
89+
##############################################################################
90+
91+
bundle agent test
92+
{
93+
meta:
94+
"description"
95+
string => concat(
96+
"Test move_obstructions with content, edit_template,",
97+
" and edit_template_string"
98+
);
99+
100+
files:
101+
# Test move_obstructions with copy_from
102+
# This isn't in init because it's specific to copy_from needing to have a source file.
103+
"$(G.testdir)/copy_from.source"
104+
content => "$(check.expected_content)",
105+
if => fileexists( "$(G.testdir)/copy_from" );
106+
107+
"$(G.testdir)/copy_from"
108+
move_obstructions => "true",
109+
copy_from => local_dcp("$(G.testdir)/copy_from.source"),
110+
if => fileexists( "$(this.promiser)" );
111+
112+
"$(G.testdir)/edit_template_cfengine"
113+
move_obstructions => "true",
114+
template_method => "cfengine",
115+
edit_template => "$(G.testdir)/copy_from.source",
116+
if => fileexists( "$(this.promiser)" );
117+
118+
"$(G.testdir)/edit_template_mustache"
119+
move_obstructions => "true",
120+
template_method => "mustache",
121+
edit_template => "$(G.testdir)/copy_from.source",
122+
if => fileexists( "$(this.promiser)" );
123+
124+
# Test move_obstructions with content attribute
125+
"$(G.testdir)/content"
126+
move_obstructions => "true",
127+
content => "$(check.expected_content)",
128+
if => fileexists( "$(this.promiser)" );
129+
130+
# Test move_obstructions with template_method inline_mustache
131+
"$(G.testdir)/edit_template_string_inline_mustache"
132+
move_obstructions => "true",
133+
template_method => "inline_mustache",
134+
edit_template_string => "$(check.expected_content)",
135+
if => fileexists( "$(this.promiser)" );
136+
137+
# # Test move_obstructions with edit_line
138+
# "$(G.testdir)/edit_line"
139+
# move_obstructions => "true",
140+
# edit_line => insert_lines("$(check.expected_content)"),
141+
# if => fileexists( "$(this.promiser)" );
142+
143+
}
144+
145+
##############################################################################
146+
147+
bundle agent check
148+
{
149+
vars:
150+
"expected_content" string => "This is content promised targeting a symlink.";
151+
"num_test_files" int => length( @(init.test_files) );
152+
153+
# We check the promised:
154+
# - for each test file
155+
# - content
156+
# - type
157+
158+
"num_expected_test_ok_classes"
159+
int => int( eval( "$(num_test_files)*2", math, infix) );
160+
161+
classes:
162+
"DEBUG" expression => "any";
163+
"all_test_files_exist"
164+
expression => filesexist( @(init.test_files) );
165+
166+
all_test_files_exist::
167+
# Once we verified that all the test files exist we can check all the expectations
168+
169+
# These class strings will be canonified as classes, they are tagged for easy identification
170+
171+
"$(init.test_files) type as expected"
172+
meta => { "test_ok_class" },
173+
if => isplain( "$(init.test_files)" );
174+
175+
"$(init.test_files) content as expected"
176+
meta => { "test_ok_class" },
177+
if => strcmp( readfile("$(init.test_files)"),
178+
"$(expected_content)");
179+
180+
"overall_success"
181+
expression => strcmp( length( classesmatching( ".*", "test_ok_class" ) ),
182+
"$(num_expected_test_ok_classes)" );
183+
184+
reports:
185+
!all_test_files_exist::
186+
"The test does not appear to have been initialized, the expected test files are missing $(with)"
187+
with => concat( "(", join( ",", @(init.test_files) ), ")" );
188+
189+
DEBUG::
190+
191+
"Number of test files: $(num_test_files)";
192+
193+
"Number of expected test ok classes to pass: $(num_expected_test_ok_classes)";
194+
195+
"$(with) test_ok_classes:"
196+
with => length( classesmatching( ".*", "test_ok_class" ) );
197+
198+
"$(with)"
199+
with => join( "$(const.n)", classesmatching( ".*", "test_ok_class" ) );
200+
201+
overall_success::
202+
"$(this.promise_filename) Pass";
203+
204+
!overall_success::
205+
"$(this.promise_filename) FAIL";
206+
}

0 commit comments

Comments
 (0)