-
Notifications
You must be signed in to change notification settings - Fork 3
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
Workload entry expiry features #96
base: main-old
Are you sure you want to change the base?
Workload entry expiry features #96
Conversation
23c839f
to
bbcc590
Compare
265d67d
to
3100b66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than these, lgtm!
}); | ||
} | ||
|
||
isValidJSTime(seconds) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not just isValidJSTime
since it asserts >0, so this is more specific to the usecase, so isValidExpiryTime
may be more appropriate, also while checking this, it may be good to also see if there's an upper limit on SPIRE type as well, i think it probably is 64 bit so just verify that that upper bound is checked too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so from whatever I could find/tell, you're right, that SPIRE deals with 64bit for time. Then, the upper limit on SPIRE is a bit larger (about .5e15) than the JS upper bound on dates - is this still then a necessary check?
@@ -115,6 +115,9 @@ class SpiffeHelper extends Component { | |||
// n:1, this would reduce the total cost. This may be useful when | |||
// performance is impacted. | |||
getAgentsEntries (agents, entries) { | |||
if (typeof entries === 'undefined') { | |||
return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im kind of curious what triggered this, this is actually a fault in the caller making this call, so we should really resolve that instead, can we also console.log
a message here, and be like "getAgentEntries: this should not happen, entries undefined"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I remove the check then? I found the error (in dashboard when there are no workload entries) and have added a todo statement in the relevant line of code in dashbard/agents-dashboard-table.js
8273a34
to
ef05c9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to be merged! Small tweaks can be done after the repo move.
722f5db
to
e2572b7
Compare
Signed-off-by: Maia Iyer <[email protected]>
e2572b7
to
58056c6
Compare
editing entry-create form for multiple entry formats and warnings on already expired entries or data that causes no expiry date