Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
772862b
Updated implementation of secrets API (without harness for now)
benhoyt Nov 28, 2022
fb5deb8
Add Secret.set_info(); make Secret.remove_revision() revision required
benhoyt Nov 30, 2022
937bf4a
Make secret_set require id param; Revision only on secret-owner events
benhoyt Dec 1, 2022
7756a8d
Very basic version of reworked secrets harness; a fair bit to do yet
benhoyt Dec 2, 2022
95cdf7c
Add more of testing harness: content, grants, revisions, hooks
benhoyt Dec 5, 2022
87934fb
Remove unnecessary expire and rotate args to Harness.add_secret
benhoyt Dec 5, 2022
b33e949
Add error messages for SecretNotFoundError
benhoyt Dec 5, 2022
7541d59
Remove secret grants when relation is removed
benhoyt Dec 5, 2022
7767b47
Remove tests on versions 3.5 and 3.6
benhoyt Dec 5, 2022
4c8e656
Doc fixes
benhoyt Dec 5, 2022
f706db0
Refactor SecretEvent types so _revision is only where needed
benhoyt Dec 6, 2022
30ece7d
Link to juju/juju#14916; make JujuVersion.has_secrets require 3.0.3+
benhoyt Dec 6, 2022
8cc6d3f
Make error message slightly closer to real backend
benhoyt Dec 6, 2022
1b19651
Fix set_info comment
benhoyt Dec 6, 2022
a382a98
Clarify what grant/revoke do if redundant
benhoyt Dec 6, 2022
86ed899
Rename _run_secret to _run_for_secret
benhoyt Dec 6, 2022
5ce4a40
Adjust _validate_content output
benhoyt Dec 6, 2022
ae434e0
Call _validate_content in harness set_secret_content
benhoyt Dec 6, 2022
998114a
Add TODO about ensuring we're not owner in set_secret_content
benhoyt Dec 6, 2022
59a0273
Improve _validate_content invalid keys error message
benhoyt Dec 7, 2022
c4d4138
Don't use shared _run_for_secret inside sensitive secret_get method
benhoyt Dec 7, 2022
5fd149f
Rename Harness.add_secret to add_model_secret
benhoyt Dec 7, 2022
720518d
Rename emit* to trigger* and tweak semantics per discussion
benhoyt Dec 7, 2022
7d97a97
Add runtime type check for "expire" parameters per code review
benhoyt Dec 7, 2022
665bc7c
Rearrange secret event classes (and harness) to reflect Juju-reality
benhoyt Dec 7, 2022
f1ae48f
Don't TypeError when expire is None; move expire check to common func
benhoyt Dec 7, 2022
6647e35
Move _parse_timestamp out of pebble.py to common place for reuse
benhoyt Dec 7, 2022
77ee815
Ensure charm doesn't own secret in set_secret_content etc
benhoyt Dec 7, 2022
a2fb674
Add basic unit tests for get_secret and add_secret(s)
benhoyt Dec 8, 2022
3e0b838
Fix pebble_cli after _parse_timestamp was moved
benhoyt Dec 9, 2022
c9df5ee
Merge branch 'main' into secrets
benhoyt Dec 9, 2022
4f29530
Fix bug with Secret.set_info setting new label
benhoyt Dec 9, 2022
11acdf3
Add tests of secret event firing
benhoyt Dec 9, 2022
a00ad75
Add tests for SecretInfo and Secret methods
benhoyt Dec 9, 2022
e983af5
Replace MockSecretBackend with existing fake_script helpers
benhoyt Dec 9, 2022
d83b200
Remove now-unused "import collections"
benhoyt Dec 9, 2022
726f032
Test SecretNotFoundError handling
benhoyt Dec 12, 2022
72f9335
Fix copying issue in add_secret docstring: never expire -> never rotate
benhoyt Dec 12, 2022
f95e81f
Rename Secret.remove_all to .remove_all_revisions for clarity
benhoyt Dec 12, 2022
6e67361
Name app_or_unit args more specifically; tweak add_model_secret comment
benhoyt Dec 12, 2022
3e981b3
Use dict comprehension; fix comment references to add_model_secret
benhoyt Dec 13, 2022
cf2352e
Add tests for the testing harness
benhoyt Dec 13, 2022
50a0131
Rename consumer -> observer per spec discussion
benhoyt Dec 13, 2022
30736a9
Rename strconv.parse_go_timestamp -> strconv.parse_rfc3339
benhoyt Dec 13, 2022
fe9a181
Rename _canonicalize_expire_time to shorter _calculate_expiry
benhoyt Dec 13, 2022
f18db12
Change _canonicalize_id to a static method, as cls is not needed
benhoyt Dec 13, 2022
a8d089c
Remove no-longer-true comment
benhoyt Dec 14, 2022
4f7e101
Note that get_content()'s refresh arg is only meaningful for observers
benhoyt Dec 14, 2022
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
4 changes: 2 additions & 2 deletions .github/workflows/framework-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
python-version: ["3.5", "3.6", "3.8", "3.10"]
python-version: ["3.8", "3.10"]

steps:
- uses: actions/checkout@v2
Expand All @@ -58,7 +58,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
python-version: ["3.5", "3.6", "3.8", "3.10"]
python-version: ["3.8", "3.10"]

steps:
- uses: actions/checkout@v2
Expand Down
55 changes: 55 additions & 0 deletions ops/_private/timeconv.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Copyright 2022 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Time conversion utilities."""

import datetime
import re

# Matches yyyy-mm-ddTHH:MM:SS(.sss)ZZZ
_TIMESTAMP_RE = re.compile(
r'(\d{4})-(\d{2})-(\d{2})[Tt](\d{2}):(\d{2}):(\d{2})(\.\d+)?(.*)')

# Matches [-+]HH:MM
_TIMEOFFSET_RE = re.compile(r'([-+])(\d{2}):(\d{2})')


def parse_rfc3339(s: str) -> datetime.datetime:
"""Parse an RFC3339 timestamp.

This parses RFC3339 timestamps (which are a subset of ISO8601 timestamps)
that Go's encoding/json package produces for time.Time values.

Unfortunately we can't use datetime.fromisoformat(), as that does not
support more than 6 digits for the fractional second, nor the 'Z' for UTC.
"""
match = _TIMESTAMP_RE.match(s)
if not match:
raise ValueError('invalid timestamp {!r}'.format(s))
y, m, d, hh, mm, ss, sfrac, zone = match.groups()

if zone in ('Z', 'z'):
tz = datetime.timezone.utc
else:
match = _TIMEOFFSET_RE.match(zone)
if not match:
raise ValueError('invalid timestamp {!r}'.format(s))
sign, zh, zm = match.groups()
tz_delta = datetime.timedelta(hours=int(zh), minutes=int(zm))
tz = datetime.timezone(tz_delta if sign == '+' else -tz_delta)

microsecond = round(float(sfrac or '0') * 1000000)

return datetime.datetime(int(y), int(m), int(d), int(hh), int(mm), int(ss),
microsecond=microsecond, tzinfo=tz)
144 changes: 143 additions & 1 deletion ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ class ConfigChangedEvent(HookEvent):
rescheduling, on unit upgrade/refresh, etc...
- As a specific instance of the above point: when networking changes
(if the machine reboots and comes up with a different IP).
- When the cloud admin reconfigures the charm via the juju CLI, i.e.
- When the cloud admin reconfigures the charm via the Juju CLI, i.e.
`juju config my-charm foo=bar`. This event notifies the charm of
its new configuration. (The event itself, however, is not aware of *what*
specifically has changed in the config).
Expand Down Expand Up @@ -742,6 +742,143 @@ class PebbleReadyEvent(WorkloadEvent):
"""


class SecretEvent(HookEvent):
"""Base class for all secret events."""

def __init__(self, handle: 'Handle', id: str, label: Optional[str]):
super().__init__(handle)
self._id = id
self._label = label

@property
def secret(self) -> model.Secret:
"""The secret instance this event refers to."""
backend = self.framework.model._backend
return model.Secret(backend=backend, id=self._id, label=self._label)

def snapshot(self) -> '_SerializedData':
"""Used by the framework to serialize the event to disk.

Not meant to be called by charm code.
"""
return {'id': self._id, 'label': self._label}

def restore(self, snapshot: '_SerializedData'):
"""Used by the framework to deserialize the event from disk.

Not meant to be called by charm code.
"""
self._id = cast(str, snapshot['id'])
self._label = cast(Optional[str], snapshot['label'])


class SecretChangedEvent(SecretEvent):
"""Event raised by Juju on the observer when the secret owner changes its contents.

When the owner of a secret changes the secret's contents, Juju will create
a new secret revision, and all applications or units that are tracking this
secret will be notified via this event that a new revision is available.

Typically, you will want to fetch the new content by calling
:meth:`ops.model.Secret.get_content` with :code:`refresh=True` to tell Juju to
start tracking the new revision.
"""


class SecretRotateEvent(SecretEvent):
"""Event raised by Juju on the owner when the secret's rotation policy elapses.

This event is fired on the secret owner to inform it that the secret must
be rotated. The event will keep firing until the owner creates a new
revision by calling :meth:`ops.model.Secret.set_content`.
"""

def defer(self):
"""Secret rotation events are not deferrable (Juju handles re-invocation)."""
raise RuntimeError(
'Cannot defer secret rotation events. Juju will keep firing this '
'event until you create a new revision.')


class SecretRemoveEvent(SecretEvent):
"""Event raised by Juju on the owner when a secret revision can be removed.

When the owner of a secret creates a new revision, and after all
observers have updated to that new revision, this event will be fired to
inform the secret owner that the old revision can be removed.

Typically, you will want to call :meth:`ops.model.Secret.remove_revision` to
remove the now-unused revision.
"""

def __init__(self, handle: 'Handle', id: str, label: Optional[str], revision: int):
super().__init__(handle, id, label)
self._revision = revision
Copy link
Member

Choose a reason for hiding this comment

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

Since the next three all just add _revision, should they have a common "SecretOwnerEvent" or "SecretRevisionEvent" ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually had SecretOwnerEvent previously, but it's actually not all owner events (SecretRotateEvent doesn't have a revision). I tried adding a _SecretEventWithRevision just now, and it works, but doesn't show up nicely in the docs: I don't want that class part of the public API, but then the docs generator doesn't show it at all and you can't see that those two subclasses have a .revision. Hmmm, going to punt for now (can always tweak later).


@property
def revision(self) -> int:
"""The secret revision this event refers to."""
return self._revision

def snapshot(self) -> '_SerializedData':
"""Used by the framework to serialize the event to disk.

Not meant to be called by charm code.
"""
data = super().snapshot()
data['revision'] = self._revision
return data

def restore(self, snapshot: '_SerializedData'):
"""Used by the framework to deserialize the event from disk.

Not meant to be called by charm code.
"""
super().restore(snapshot)
self._revision = cast(int, snapshot['revision'])


class SecretExpiredEvent(SecretEvent):
"""Event raised by Juju on the owner when a secret's expiration time elapses.

This event is fired on the secret owner to inform it that the secret revision
must be removed. The event will keep firing until the owner removes the
revision by calling :meth:`model.Secret.remove_revision()`.
"""

def __init__(self, handle: 'Handle', id: str, label: Optional[str], revision: int):
super().__init__(handle, id, label)
self._revision = revision

@property
def revision(self) -> int:
"""The secret revision this event refers to."""
return self._revision

def snapshot(self) -> '_SerializedData':
"""Used by the framework to serialize the event to disk.

Not meant to be called by charm code.
"""
data = super().snapshot()
data['revision'] = self._revision
return data

def restore(self, snapshot: '_SerializedData'):
"""Used by the framework to deserialize the event from disk.

Not meant to be called by charm code.
"""
super().restore(snapshot)
self._revision = cast(int, snapshot['revision'])

def defer(self):
"""Secret expiration events are not deferrable (Juju handles re-invocation)."""
raise RuntimeError(
'Cannot defer secret expiration events. Juju will keep firing '
'this event until you create a new revision.')


class CharmEvents(ObjectEvents):
"""Events generated by Juju pertaining to application lifecycle.

Expand Down Expand Up @@ -783,6 +920,11 @@ class CharmEvents(ObjectEvents):
leader_settings_changed = EventSource(LeaderSettingsChangedEvent)
collect_metrics = EventSource(CollectMetricsEvent)

secret_changed = EventSource(SecretChangedEvent)
secret_expired = EventSource(SecretExpiredEvent)
secret_rotate = EventSource(SecretRotateEvent)
secret_remove = EventSource(SecretRemoveEvent)


class CharmBase(Object):
"""Base class that represents the charm overall.
Expand Down
11 changes: 8 additions & 3 deletions ops/jujuversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,23 @@ def from_environ(cls) -> 'JujuVersion':
return cls(v)

def has_app_data(self) -> bool:
"""Determine whether this juju version knows about app data."""
"""Determine whether this Juju version knows about app data."""
return (self.major, self.minor, self.patch) >= (2, 7, 0)

def is_dispatch_aware(self) -> bool:
"""Determine whether this juju version knows about dispatch."""
"""Determine whether this Juju version knows about dispatch."""
return (self.major, self.minor, self.patch) >= (2, 8, 0)

def has_controller_storage(self) -> bool:
"""Determine whether this juju version supports controller-side storage."""
"""Determine whether this Juju version supports controller-side storage."""
return (self.major, self.minor, self.patch) >= (2, 8, 0)

@property
def has_secrets(self) -> bool:
"""Determine whether this Juju version supports the `secrets` feature."""
# Juju version 3.0.0 had an initial version of secrets, but:
# * In 3.0.2, secret-get "--update" was renamed to "--refresh", and
# secret-get-info was separated into its own hook tool
# * In 3.0.3, a bug with observer labels was fixed (juju/juju#14916)
# TODO(benhoyt): update to 3.0.3+ once shipped (for juju/juju#14916 fix)
return (self.major, self.minor, self.patch) >= (3, 0, 2)
12 changes: 10 additions & 2 deletions ops/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def _setup_event_links(charm_dir: Path, charm: 'CharmBase'):
"""
# XXX: on windows this function does not accomplish what it wants to:
# it creates symlinks with no extension pointing to a .py
# and juju only knows how to handle .exe, .bat, .cmd, and .ps1
# and Juju only knows how to handle .exe, .bat, .cmd, and .ps1
# so it does its job, but does not accomplish anything as the
# hooks aren't 'callable'.
link_to = os.path.realpath(os.environ.get("JUJU_DISPATCH_PATH", sys.argv[0]))
Expand Down Expand Up @@ -160,6 +160,14 @@ def _get_event_args(charm: 'CharmBase',
workload_name = os.environ['JUJU_WORKLOAD_NAME']
container = model.unit.get_container(workload_name)
return [container], {}
elif issubclass(event_type, ops.charm.SecretEvent):
args: List[Any] = [
os.environ['JUJU_SECRET_ID'],
os.environ.get('JUJU_SECRET_LABEL'),
]
if issubclass(event_type, (ops.charm.SecretRemoveEvent, ops.charm.SecretExpiredEvent)):
args.append(int(os.environ['JUJU_SECRET_REVISION']))
return args, {}
elif issubclass(event_type, ops.charm.StorageEvent):
storage_id = os.environ.get("JUJU_STORAGE_ID", "")
if storage_id:
Expand Down Expand Up @@ -399,7 +407,7 @@ def main(charm_class: Type[ops.charm.CharmBase],
if use_juju_for_storage:
if dispatcher.is_restricted_context():
# TODO: jam 2020-06-30 This unconditionally avoids running a collect metrics event
# Though we eventually expect that juju will run collect-metrics in a
# Though we eventually expect that Juju will run collect-metrics in a
# non-restricted context. Once we can determine that we are running collect-metrics
# in a non-restricted context, we should fire the event as normal.
logger.debug('"%s" is not supported when using Juju for storage\n'
Expand Down
Loading