Skip to content

CFE-2815: Added isnewerthantime function #5799

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.

Sign up for GitHub

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

Merged
nickanderson merged 2 commits into from
May 28, 2025
Merged

CFE-2815: Added isnewerthantime function #5799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5f43cb
Remove redundant check in a test
jakub-nt May 16, 2025
e497257
Added isnewerthantime policy function
jakub-nt May 16, 2025
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
2 changes: 1 addition & 1 deletion examples/isnewerthan.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ bundle agent example
{
classes:

"do_it" and => { isnewerthan("/tmp/later","/tmp/earlier"), "cfengine" };
"do_it" expression => isnewerthan("/tmp/later","/tmp/earlier");
Comment thread
jakub-nt marked this conversation as resolved.

reports:

Expand Down
33 changes: 33 additions & 0 deletions examples/isnewerthantime.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#+begin_src prep
#@ ```
#@ touch -t '200102031234.56' /tmp/file_a
#@ touch -t '200202031234.56' /tmp/file_b
#@ ```
#+end_src
###############################################################################
#+begin_src cfengine3
body common control
{
bundlesequence => { "example" };
}

bundle agent example
{
classes:
"is_file_a_new" expression => isnewerthantime("/tmp/file_a", 1000000000);
"is_file_b_new" expression => isnewerthantime("/tmp/file_b", 1000000000);

reports:
!is_file_a_new::
"/tmp/file_a is not newer than 2001-09-09 01:46:40";
is_file_b_new::
"/tmp/file_b is newer than 2001-09-09 01:46:40";
}
#+end_src
###############################################################################
#+begin_src example_output
#@ ```
#@ R: /tmp/file_a is not newer than 2001-09-09 01:46:40
#@ R: /tmp/file_b is newer than 2001-09-09 01:46:40
#@ ```
#+end_src
35 changes: 33 additions & 2 deletions libpromises/evalfunction.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3068,7 +3068,7 @@
}

return FnReturnContext(sd > -1);
}
}

/*********************************************************************/

Expand Down Expand Up @@ -4496,6 +4496,28 @@
return FnReturnContext(frombuf.st_mtime > tobuf.st_mtime);
}

static FnCallResult FnCallIsNewerThanTime(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Policy *policy, ARG_UNUSED const FnCall *fp, const Rlist *finalargs)
{
Comment thread
jakub-nt marked this conversation as resolved.
Comment thread
jakub-nt marked this conversation as resolved.
assert(finalargs != NULL);

const char *arg_file = RlistScalarValue(finalargs);
// a comment in `FnCallStrftime`: "this will be a problem on 32-bit systems..."
const time_t arg_mtime = IntFromString(RlistScalarValue(finalargs->next));

struct stat file_buf;
int exit_code = stat(arg_file, &file_buf);
if (exit_code == -1)
{

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 162 lines.
return FnFailure();

@victormlg victormlg May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe you could add some user friendly error here?

@olehermanse olehermanse May 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general it's a good idea to add helpful error messages, but looking at the other similar policy functions below, this seems consistent with them - they also return FnFailure() without printing an error, when stat fails, indicating:

  1. Maybe it's desirable to not print anything? Would adding errors become too noisy?
  2. Maybe it's intentional, since something else is responsible for printing the error automatically when you return FnFailure()?
  3. Maybe it is really a mistake, and we should update all of these to have error messages?

I haven't looked into it further, so I don't know the answer. If you find out, and it's 1. or 2., probably a comment would be nice instead, or a debug level message, or both.

}

time_t file_mtime = file_buf.st_mtime;

bool result = file_mtime > arg_mtime;

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter finalargs in FnCallSelectServers() is dereferenced without an explicit null-check

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter finalargs in FnCallSelectServers() is dereferenced without an explicit null-check
return FnReturnContext(result);

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter finalargs in FnCallSelectServers() is dereferenced without an explicit null-check
}

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter finalargs in FnCallSelectServers() is dereferenced without an explicit null-check

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter finalargs in FnCallSelectServers() is dereferenced without an explicit null-check
/*********************************************************************/

static FnCallResult FnCallIsAccessedBefore(ARG_UNUSED EvalContext *ctx, ARG_UNUSED const Policy *policy, ARG_UNUSED const FnCall *fp, const Rlist *finalargs)
Expand Down Expand Up @@ -7349,7 +7371,7 @@
{
isvalid = isvalid && JsonGetElementType(json) != JSON_ELEMENT_TYPE_PRIMITIVE;
}

FnCallResult ret = FnReturnContext(isvalid);
JsonDestroy(json);
return ret;
Expand Down Expand Up @@ -9840,6 +9862,13 @@
{NULL, CF_DATA_TYPE_NONE, NULL}
};

static const FnCallArg FILE_TIME_ARGS[] =
{
{CF_ABSPATHRANGE, CF_DATA_TYPE_STRING, "File name"},
{CF_VALRANGE, CF_DATA_TYPE_INT, "Time as a Unix epoch offset"},
{NULL, CF_DATA_TYPE_NONE, NULL}
};

static const FnCallArg ISVARIABLE_ARGS[] =
{
{CF_ANYSTRING, CF_DATA_TYPE_STRING, "Variable identifier"},
Expand Down Expand Up @@ -10677,6 +10706,8 @@
FNCALL_OPTION_NONE, FNCALL_CATEGORY_FILES, SYNTAX_STATUS_NORMAL),
FnCallTypeNew("isnewerthan", CF_DATA_TYPE_CONTEXT, ISNEWERTHAN_ARGS, &FnCallIsNewerThan, "True if arg1 is newer (modified later) than arg2 (mtime)",
FNCALL_OPTION_NONE, FNCALL_CATEGORY_FILES, SYNTAX_STATUS_NORMAL),
FnCallTypeNew("isnewerthantime", CF_DATA_TYPE_CONTEXT, FILE_TIME_ARGS, &FnCallIsNewerThanTime, "True if arg1 is newer (modified later) (has larger mtime) than time arg2",
FNCALL_OPTION_NONE, FNCALL_CATEGORY_FILES, SYNTAX_STATUS_NORMAL),
FnCallTypeNew("isplain", CF_DATA_TYPE_CONTEXT, FILESTAT_ARGS, &FnCallFileStat, "True if the named object is a plain/regular file",
FNCALL_OPTION_NONE, FNCALL_CATEGORY_FILES, SYNTAX_STATUS_NORMAL),
FnCallTypeNew("isvariable", CF_DATA_TYPE_CONTEXT, ISVARIABLE_ARGS, &FnCallIsVariable, "True if the named variable is defined",
Expand Down
64 changes: 64 additions & 0 deletions tests/acceptance/02_classes/02_functions/isnewerthantime.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
body common control
{
inputs => { "../../default.cf.sub" };
bundlesequence => { default("$(this.promise_filename)") };
Comment on lines +3 to +4

@larsewi larsewi May 22, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since you are not using anything from default.cf.sub you can consider not including it and instead write your own bundle sequence.

Suggested change
inputs => { "../../default.cf.sub" };
bundlesequence => { default("$(this.promise_filename)") };
bundlesequence => { "check" };

@nickanderson nickanderson May 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alternatively, you can omit body common control and just have bundle agent main and then use methods promises to call each of the other bundles you want.

bundle agent __main__
{
  methods:
    "init";
    "test";
    "check";
    "cleanup";

So, many ways to skin the cat.

version => "1.0";
}
#######################################################

bundle agent check
{
vars:
"test_file"
string => "$(this.promise_dirname)/isnewerthantime_file.txt";

@larsewi larsewi May 22, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can consider creating this file in an init bundle and setting the mtime, all using the files promise.

@nickanderson nickanderson May 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Implementation is using the derived mtime via filestat()

"nonexistent_file"
string => "$(this.promise_dirname)/thisfiledoesntexist_1747148781.txt";
"test_file_mtime"
string => filestat("$(test_file)", mtime);
"test_file_time_just_before_mtime"
int => int(eval("$(test_file_mtime) - 1"));
"minus_one"
int => "-1";
classes:
"ok1"
expression => isnewerthantime("$(test_file)", 0);
"ok2"
expression => isnewerthantime("$(nonexistent_file)", 0);
"ok3"
expression => isnewerthantime("$(test_file)", "$(test_file_time_just_before_mtime)");
"ok4"
expression => isnewerthantime("$(test_file)", "$(test_file_mtime)");
"ok5"
expression => isnewerthantime("$(nonexistent_file)", "$(test_file_mtime)");
"ok6"
# as "inf" corresponds to 999999999, this is actually not False
expression => isnewerthantime("$(test_file)", "inf");
"ok7"
expression => isnewerthantime("$(nonexistent_file)", "inf");
"ok8"
expression => isnewerthantime("$(test_file)", "$(minus_one)");
"ok"
expression => and("ok1", "!ok2", "ok3", "!ok4", "!ok5", "ok6", "!ok7", "ok8");

reports:
DEBUG.ok1::
"1. pass";
DEBUG.!ok2::
"2. pass";
DEBUG.ok3::
"3. pass: $(test_file_time_just_before_mtime)";
DEBUG.!ok4::
"4. pass: $(test_file_mtime)";
DEBUG.!ok5::
"5. pass";
DEBUG.ok6::
"6. pass";
DEBUG.!ok7::
"7. pass";
DEBUG.ok8::
"8. pass";
ok::
"$(this.promise_filename) Pass";
!ok::
"$(this.promise_filename) FAIL";
}
Empty file added tests/acceptance/02_classes/02_functions/isnewerthantime_file.txt
View file
Open in desktop
Empty file.
Loading