Added a time_add() filter to add duration and absolute time #5817

vishal-chdhry · 2022-12-30T06:38:27Z

Signed-off-by: Vishal Choudhary contactvishaltech@gmail.com

Explanation

Adds a custom JMESPath filter to support inputs in the form of RFC3339 + scalar (duration) with the sum being RFC3339.

Related issue

Closes #5815

Milestone of this PR

What type of PR is this

/kind feature

Proposed Changes

Proof Manifests

Checklist

I have read the contributing guidelines.
I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
This is a bug fix and I have added unit tests that prove my fix is effective.
This is a feature and I have added CLI tests that are applicable.
My PR needs to be cherry picked to a specific release branch which is .
My PR contains new or altered behavior to Kyverno and
- CLI support should be added and my PR doesn't contain that functionality.
- I have added or changed the documentation myself in an existing PR and the link is:
- I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

codecov · 2022-12-30T06:46:44Z

Codecov Report

Merging #5817 (11046fb) into main (c10649f) will increase coverage by 0.10%.
The diff coverage is 65.71%.

@@            Coverage Diff             @@
##             main    #5817      +/-   ##
==========================================
+ Coverage   35.75%   35.85%   +0.10%     
==========================================
  Files         188      188              
  Lines       20884    20954      +70     
==========================================
+ Hits         7467     7513      +46     
- Misses      12598    12614      +16     
- Partials      819      827       +8

Impacted Files	Coverage Δ
pkg/engine/jmespath/functions.go	`71.81% <65.71%> (-0.49%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

eddycharly · 2023-01-02T16:00:04Z

I don't really like the format argument, how about a time_convert filter ?
cc @chipzoller

realshuting · 2023-01-03T13:26:12Z

I don't really like the format argument, how about a time_convert filter ? cc @chipzoller

+1, time_convert seems more clear to me.

chipzoller · 2023-01-03T13:30:15Z

The arguments here are similar to time_since(). I don't mind the idea of this time_add() filter being able to accommodate multiple time formats, but if that's not working here then it either needs to be fixed, removed and the filter renamed, or something else.

eddycharly · 2023-01-03T13:51:51Z

IMHO we should support only one format internally and use conversion when input is in another format.

eddycharly · 2023-01-05T08:10:02Z

pkg/engine/jmespath/functions.go

@@ -461,6 +462,18 @@ func GetFunctions() []*FunctionEntry {
 			ReturnType: []JpType{JpObject},
 			Note:       "decodes an x.509 certificate to an object. you may also use this in conjunction with `base64_decode` jmespath function to decode a base64-encoded certificate",
 		},
+		{


@vishal-chdhry can you add a note to the function descriptor ?

realshuting · 2023-01-05T12:43:34Z

@vishal-chdhry - can you address comment here? We'd like to get this merged to 1.9 and cut beta.2 by Friday.

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

vishal-chdhry · 2023-01-05T16:37:10Z

@realshuting @eddycharly done!

chipzoller · 2023-01-05T16:38:08Z

Thanks for these, @vishal-chdhry !

eddycharly · 2023-01-05T17:06:35Z

Thanks @vishal-chdhry !

We were discussing the possibility to add a time_convert filter to allow removing the format argument from all related filters.
What do you think ?
Do you want to work on this ? Or we can merge this PR and take care of it in follow up PRs ?

vishal-chdhry · 2023-01-05T17:08:48Z

@eddycharly I can add it to this PR

eddycharly · 2023-01-05T17:09:48Z

@vishal-chdhry it would be awesome ❤️

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

vishal-chdhry · 2023-01-05T17:14:13Z

should I convert the times to utc or keep the given timezone? (currently time_convert preserves the timezone)
@eddycharly can you please approve the change so that I can add the changes to existing functions

eddycharly · 2023-01-05T17:25:05Z

should I convert the times to utc or keep the given timezone? (currently time_convert preserves the timezone)

@vishal-chdhry isn't it covered by the format ?

can you please approve the change so that I can add the changes to existing functions

we don't want to change time_since as it would be a breaking change, we can add a new time_diff func and deprecate time_since instead.

eddycharly · 2023-01-05T17:26:19Z

pkg/engine/jmespath/functions_test.go

+		expectedResult string
+	}{
+		{
+			test:           "time_convert('', '2021-01-02T15:04:05-07:00')",


Does it make sense to call time_convert with an empty format ?

empty string everywhere else means we are using RFC3339, I kept it for consistency

and if the layout is empty and the time isn't in RFC3339 format then it will throw an error

I understand the idea but why one would call time_convert if the input is already in RFC3339 ?

I would rather not default the format, sounds error prone.

ok, I should remove it then

vishal-chdhry · 2023-01-05T17:30:28Z

isn't it covered by the format ?

@eddycharly time.format() doesn't change the timezone to UTC, for example, in the first test case

eddycharly · 2023-01-05T17:36:08Z

time.format() doesn't change the timezone to UTC

what are the pros/cons of converting to UTC ?

do you have an opinion @JimBugwadia @chipzoller @realshuting ?

eddycharly · 2023-01-05T17:36:52Z

@vishal-chdhry what would be the best name time_convert or time_parse ?

vishal-chdhry · 2023-01-05T17:59:55Z

@eddycharlytime_parse maybe because we are not converting time, just parsing it and its the same value in the end

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

chipzoller · 2023-01-05T20:08:00Z

time.format() doesn't change the timezone to UTC

what are the pros/cons of converting to UTC ?

do you have an opinion @JimBugwadia @chipzoller @realshuting ?

The pros are pretty clear:

Consistent time format
Avoids potential problems with multiple replicas on hosts in different time zones

Cons are less clear but seem to be few from what I can think of.

eddycharly · 2023-01-06T12:39:39Z

IMHO it's better to not enforce UTC.
We need RFC3339 format but UTC is not mandatory.

eddycharly · 2023-01-06T12:40:50Z

@vishal-chdhry can you resolve conflicts ?

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

vishal-chdhry · 2023-01-06T13:39:23Z

@eddycharly Done!

chipzoller · 2023-01-06T13:41:34Z

One thing I ran into testing CronJobs is to be mindful that the cron format given is interpreted by the time zone of the Node on which the Pod will execute. In 1.26 there's a way to specify a time zone, but that's obviously not something we can adopt ourselves now.

eddycharly · 2023-01-06T13:55:16Z

@chipzoller IIRC it's the TZ of the master nodes, not the TZ of the node where the pod executes.

eddycharly

Good job @vishal-chdhry !

eddycharly · 2023-01-07T09:37:58Z

/cherry-pick release-1.9

eddycharly · 2023-01-09T14:17:43Z

/cherry-pick release-1.9

eddycharly · 2023-01-09T14:18:36Z

/cherry-pick release-1.9

…#5817) * time_add Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * output is RFC3339 only now Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added note to timeadd Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added time_convert Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * removed blank string timezone Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * renamed to time_parse Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

…5946) * time_add Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * output is RFC3339 only now Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added note to timeadd Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added time_convert Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * removed blank string timezone Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * renamed to time_parse Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> Co-authored-by: Vishal Choudhary <contactvishaltech@gmail.com> Co-authored-by: shuting <shuting@nirmata.com>

…#5817) * time_add Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * output is RFC3339 only now Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added note to timeadd Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * added time_convert Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * removed blank string timezone Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> * renamed to time_parse Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> Signed-off-by: MdSahil-oss <Mohdssahil1@gmail.com>

vishal-chdhry added 2 commits December 30, 2022 12:01

time_add

036801d

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

output is RFC3339 only now

acbb741

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

vishal-chdhry requested a review from prateekpandey14 as a code owner December 30, 2022 06:38

vishal-chdhry mentioned this pull request Dec 31, 2022

[1.9] Add documentation for time_now, time_parse, time_to_cron and time_add JMESPath filters kyverno/website#741

Closed

realshuting added the milestone 1.9.0 label Jan 4, 2023

realshuting added this to the Kyverno Release 1.9.0 milestone Jan 4, 2023

eddycharly reviewed Jan 5, 2023

View reviewed changes

added note to timeadd

f8e9bba

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

added time_convert

246e95a

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

eddycharly reviewed Jan 5, 2023

View reviewed changes

vishal-chdhry added 2 commits January 5, 2023 23:30

removed blank string timezone

ad70fef

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

renamed to time_parse

86388ed

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

Merge branch 'main' into jmes-time-add

db47446

Merge branch 'main' into jmes-time-add

a8b4a74

Signed-off-by: Vishal Choudhary <contactvishaltech@gmail.com>

Merge branch 'main' into jmes-time-add

895a77a

eddycharly approved these changes Jan 6, 2023

View reviewed changes

eddycharly enabled auto-merge (squash) January 6, 2023 13:55

Merge branch 'main' into jmes-time-add

2b52109

eddycharly added the cherry-pick-required label Jan 6, 2023

Merge branch 'main' into jmes-time-add

11046fb

eddycharly merged commit bb6005a into kyverno:main Jan 6, 2023

eddycharly mentioned this pull request Jan 9, 2023

Added a time_add() filter to add duration and absolute time (cherry-pick #5817) #5946

Merged

eddycharly added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Jan 9, 2023

chipzoller mentioned this pull request Jan 9, 2023

Allow time-based window for policy enforcement behavior #2233

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a time_add() filter to add duration and absolute time #5817

Added a time_add() filter to add duration and absolute time #5817

vishal-chdhry commented Dec 30, 2022

codecov bot commented Dec 30, 2022 •

edited

eddycharly commented Jan 2, 2023 •

edited

realshuting commented Jan 3, 2023

chipzoller commented Jan 3, 2023

eddycharly commented Jan 3, 2023

eddycharly Jan 5, 2023

realshuting commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023

chipzoller commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023 •

edited

eddycharly commented Jan 5, 2023

eddycharly Jan 5, 2023

vishal-chdhry Jan 5, 2023

eddycharly Jan 5, 2023

eddycharly Jan 5, 2023

vishal-chdhry Jan 5, 2023

vishal-chdhry commented Jan 5, 2023

eddycharly commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023 •

edited

chipzoller commented Jan 5, 2023

eddycharly commented Jan 6, 2023

eddycharly commented Jan 6, 2023

vishal-chdhry commented Jan 6, 2023

chipzoller commented Jan 6, 2023

eddycharly commented Jan 6, 2023

eddycharly left a comment

eddycharly commented Jan 7, 2023

eddycharly commented Jan 9, 2023

eddycharly commented Jan 9, 2023

Added a time_add() filter to add duration and absolute time #5817

Added a time_add() filter to add duration and absolute time #5817

Conversation

vishal-chdhry commented Dec 30, 2022

Explanation

Related issue

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

Further Comments

codecov bot commented Dec 30, 2022 • edited

Codecov Report

eddycharly commented Jan 2, 2023 • edited

realshuting commented Jan 3, 2023

chipzoller commented Jan 3, 2023

eddycharly commented Jan 3, 2023

eddycharly Jan 5, 2023

Choose a reason for hiding this comment

realshuting commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023

chipzoller commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023 • edited

eddycharly commented Jan 5, 2023

eddycharly Jan 5, 2023

Choose a reason for hiding this comment

vishal-chdhry Jan 5, 2023

Choose a reason for hiding this comment

eddycharly Jan 5, 2023

Choose a reason for hiding this comment

eddycharly Jan 5, 2023

Choose a reason for hiding this comment

vishal-chdhry Jan 5, 2023

Choose a reason for hiding this comment

vishal-chdhry commented Jan 5, 2023

eddycharly commented Jan 5, 2023

eddycharly commented Jan 5, 2023

vishal-chdhry commented Jan 5, 2023 • edited

chipzoller commented Jan 5, 2023

eddycharly commented Jan 6, 2023

eddycharly commented Jan 6, 2023

vishal-chdhry commented Jan 6, 2023

chipzoller commented Jan 6, 2023

eddycharly commented Jan 6, 2023

eddycharly left a comment

Choose a reason for hiding this comment

eddycharly commented Jan 7, 2023

eddycharly commented Jan 9, 2023

eddycharly commented Jan 9, 2023

codecov bot commented Dec 30, 2022 •

edited

eddycharly commented Jan 2, 2023 •

edited

vishal-chdhry commented Jan 5, 2023 •

edited

vishal-chdhry commented Jan 5, 2023 •

edited