Skip to content

fix: use timezone-aware datetimes in expiry calculation#2378

Merged
tonyandrewmeyer merged 6 commits intocanonical:mainfrom
tonyandrewmeyer:fix/timezone-aware-datetimes
Mar 25, 2026
Merged

fix: use timezone-aware datetimes in expiry calculation#2378
tonyandrewmeyer merged 6 commits intocanonical:mainfrom
tonyandrewmeyer:fix/timezone-aware-datetimes

Conversation

@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator

@tonyandrewmeyer tonyandrewmeyer commented Mar 15, 2026

Summary

  • _calculate_expiry() (line 539): Changed datetime.datetime.now() to datetime.datetime.now(tz=datetime.timezone.utc) so that when a charm passes a timedelta for secret expiry, the resulting datetime is timezone-aware. Previously, the naive datetime would be serialised via datetime_to_rfc3339 (which calls .isoformat()), producing a string without a timezone suffix. Juju expects RFC 3339 timestamps with timezone information.

  • Container._build_fileinfo() (line 3047): Changed datetime.datetime.fromtimestamp(info.st_mtime) to datetime.datetime.fromtimestamp(info.st_mtime, tz=datetime.timezone.utc) so that FileInfo.last_modified carries UTC timezone information rather than being a naive datetime in the local timezone.

Both fixes ensure that datetimes produced by these functions include explicit UTC timezone data, which is necessary for correct RFC 3339 serialisation and consistent behaviour across different host timezone configurations.

🤖 Generated with Claude Code but owned by me.

@tonyandrewmeyer tonyandrewmeyer force-pushed the fix/timezone-aware-datetimes branch 2 times, most recently from 9be875b to bf84978 Compare March 15, 2026 05:56
Use `datetime.datetime.now(tz=datetime.timezone.utc)` in
`_calculate_expiry()` and `datetime.datetime.fromtimestamp(...,
tz=datetime.timezone.utc)` in `Container._build_fileinfo()` so that
the resulting datetimes carry UTC timezone information. Naive datetimes
are problematic because `datetime.isoformat()` omits the timezone
suffix, producing strings that do not conform to RFC 3339 as expected
by Juju.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@james-garner-canonical james-garner-canonical left a comment

Choose a reason for hiding this comment

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

The _calculate_expiry change seems correct.

Does this fix an active bug or one that we'd hit with a future Juju release? The PR description isn't clear, e.g.

Juju expects RFC 3339 timestamps with timezone information.

Implies that our timezone-naive timezones might be being rejected or treated oddly by Juju (perhaps it treats timestamps without timezone info as UTC, which will be incorrect in most timezones?).


The _build_fileinfo change appears to be a no-op (see comment), but is more correct, so that one doesn't need discussion.

Copy link
Copy Markdown
Contributor

@dimaqq dimaqq left a comment

Choose a reason for hiding this comment

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

The change looks solid.

I'm curious how come this was not an issue until now.

Also, please validate that some old Juju, like 2.9.x honours the new timestamps. I'd assume that Juju did from the get-go, being coded in Go, but better safe than sorry.

@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator Author

Also, please validate that some old Juju, like 2.9.x honours the new timestamps. I'd assume that Juju did from the get-go, being coded in Go, but better safe than sorry.

The change is in secrets, which aren't in 2.9, so that's not something that can be validated. (The other case was internal Ops code, but has been removed as suggested elsewhere in the PR).

@tonyandrewmeyer tonyandrewmeyer changed the title fix: use timezone-aware datetimes in expiry calculation and file info fix: use timezone-aware datetimes in expiry calculation Mar 25, 2026
@tonyandrewmeyer tonyandrewmeyer merged commit 7e7c40d into canonical:main Mar 25, 2026
60 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the fix/timezone-aware-datetimes branch March 25, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants