This linter has been deprecated and is disabled. I would have thought
nolintlint would catch this, but it did not.
Signed-off-by: Nic Cope <nicc@rk0n.org>
I thought this would help with compatibility, but unfortunately this
package has breaking changes that will affect anyone who was using these
types regardless.
Signed-off-by: Nic Cope <nicc@rk0n.org>
This required some updates to our implementations of client.Client,
which now has a generic SubResource sub-client.
Signed-off-by: Nic Cope <nicc@rk0n.org>
- If a managed resource has the "crossplane.io/paused" annotation with its value
set to "true", then the managed reconciler emits an event indicating that
further reconciliations on that resource are paused and returns early after
setting a Synced status condition to false with the reason "ReconcilePaused".
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
The primary functional change here is to avoid setting a status condition when a
deferred disconnect fails. We don't want to overwrite the original status
condition that may have been written if we're returning from Reconcile because
we hit an error. Emitting an event and a debug log should be sufficient.
This commit also tweaks a bit of grammar and updates the NopConnectDisconnecter
implementation to more closely match its docstring description.
Signed-off-by: Nic Cope <negz@rk0n.org>
Updates the package parsing logic to only attempt decoding with the
object scheme in the case that the error from decoding in the meta
scheme is due to the GVK not being registered. This does not change the
definition of a valid package, but does result in more informative
errors being returned when a package is invalid due to a malformed meta
type.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
I believe the Reconcile method started including a context in controller-runtime
v0.8.0, but it was never plumbed up. If I follow the contexts correctly the one
passed to Reconcile can be traced back to the one passed to mgr.Start, which is
typically a context that is cancelled on SIGTERM or SIGINT.
Signed-off-by: Nic Cope <negz@rk0n.org>
This PR tweaks how ratelimiters are applied to support _actual_ global reconcile
rate limiting - that is all reconcile triggers are rate limited, not just some.
See https://github.com/crossplane/crossplane/issues/2595 for details.
Signed-off-by: Nic Cope <negz@rk0n.org>
I don't really expect these to be used in practice. They're mostly useful for
places like the XRD controllers where we need a default set of options to plumb
down to the XR and XRC controllers when none are passed to use (i.e. in tests).
Signed-off-by: Nic Cope <negz@rk0n.org>
This type is intended to be passed as the argument to most Crossplane Setup
functions, for example:
```go
func Setup(mgr ctrl.Manager, o controller.Options) error
```
This allows us to add new options to be plumbed down to all or most controllers
without increasing the number of arguments provided to each Setup function. Sets
of controllers that require additional arguments (e.g. the pkg controllers from
crossplane/crossplane) can define their own Options struct that embeds this one.
Signed-off-by: Nic Cope <negz@rk0n.org>
I'd like to reuse these existing ratelimiters for crossplane, where the names
'Provider' and 'Managed' don't make as much sense.
Signed-off-by: Nic Cope <negz@rk0n.org>
https://github.com/crossplane/crossplane/tree/b7ce021e32/internal/featurehttps://github.com/crossplane/crossplane/issues/2313.
This is a copy of the (almost) identical crossplane/crossplane package, which
will be removed in favor of this one. Moving to crossplane-runtime allows us to
use the same package in providers, e.g. to disable Alpha APIs per the above
issue.
The package is _almost_ identical because the Flag type has been changed from
int to string. This makes it easier to give flags string names, because the
stringer tool we previously used requires that types and instances be defined in
the same package.
Signed-off-by: Nic Cope <negz@rk0n.org>
https://github.com/crossplane/crossplane-runtime/issues/285
This approach causes us to repeat ourselves a bit, but prevents issues like the
above, where refactoring caused us to accidentally overwrite a pending status
update that we hadn't committed.
Signed-off-by: Nic Cope <negz@rk0n.org>
I tried to address this TODO today, but found that I kind of prefer how our
implementation works. I've found from time to time while writing tests that
I was accidentally wrapping my errors with the wrong context (i.e. message),
which is not something the cmpopts variant tests for.
Signed-off-by: Nic Cope <negz@rk0n.org>
Go introduced a 'native' way to wrap errors back in v1.13. At that point we were
already using github.com/pkg/errors to 'wrap' errors with context, and we never
got around to migrating. In addition to pure inertia, I've personally avoided
making the switch because I prefer the github.com/pkg/errors API. Specifically I
like that errors.Wrap handles the "outer context: inner context" error format
that Go uses by convention, and that errors.Wrap will return nil when passed a
nil error.
Given that github.com/pkg/errors has long been in maintenance mode, and is (per
https://github.com/pkg/errors/issues/245) no longer used by its original author
now seems as good a time as any to migrate. This commit attempts to ease that
migration for the Crossplane project - and to retain the nice API - by adding a
package that acts as a small github.com/pkg/errors style shim layer around the
stdlib pkg/errors (and friends, like fmt.Errorf).
Signed-off-by: Nic Cope <negz@rk0n.org>
The retry logic we use to persist critical annotations makes it difficult to
delete an annotation without potentially also deleting annotations added by
another controller (e.g. the composition logic). This commit therefore changes
the way we detect whether we might have created an external resource but not
recorded the result. Previously we relied on the presence of the 'pending'
annotation to detect this state. Now we check whether the 'pending' annotation
is newer than any 'succeeded' or 'failed' annotation.
Signed-off-by: Nic Cope <negz@rk0n.org>
This commit is intended to address two issues that we diagnosed while
investigating https://github.com/crossplane/provider-aws/issues/802.
The first issue is that controller-runtime does not guarantee reads from cache
will return the freshest version of a resource. It's possible we could create an
external resource in one reconcile, then shortly after trigger another in which
it appears that the managed resource was never created because we didn't record
its external-name. This only affects the subset of managed resources with
non-deterministic external-names that are assigned during creation.
The second issue is that some external APIs are eventually consistent. A newly
created external resource may take some time before our ExternalClient's observe
call can confirm it exists. AWS EC2 is an example of one such API.
This commit attempts to address the first issue by making an Update to a managed
resource immediately before Create it called. This Update call will be rejected
by the API server if the managed resource we read from cache was not the latest
version.
It attempts to address the second issue by allowing managed resource controller
authors to configure an optional grace period that begins when an external
resource is successfully created. During this grace period we'll requeue and
keep waiting if Observe determines that the external resource doesn't exist,
rather than (re)creating it.
Signed-off-by: Nic Cope <negz@rk0n.org>
- Move resource.WithMergeOptions to core Crossplane and unexport
- Move fieldpath.object functions to core Crossplane and unexport
- Move fieldpath.MergeValue & related functions to its own file
- Add tests for fieldpath.MergeValue
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
If a document contains only whitespace and we are not able to decode it
into one of the supported schemas, we ignore it and continue. We want to
avoid skipping a type that does not have a registered schema so that we
do not silently ignore errors when package building, but we consider it
safe to assume that a YAML document with no content is safe to skip.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
YAML supports a document terminator (...) when reading files in a
stream. This allows for the reader to indicate that the current document
has ended without indicating that the stream is finished. Anything after
the document terminator is ignored by the consumer. Therefore, we write
the terminator at the end of a read file, then immediately follow with a
YAML document separator (---), which is mandatory according to the YAML
spec when using document terminators.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Updates managed reconciler unit tests to reflect that managed resources
with a deletion policy of Orphan will have connection details
unpublished and finalizer removed without attempting to connect to
external client.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Now that the managed reconciler will handle deletion with policy Orphan
early, there is no need to check that deletion policy is not orphan when
handling a managed resource with deletion timestamp set and external
resource exists.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Updates the status of managed resource to be set to deleting in the case
where an external resource does not exist, but we fail to unpublish
connection or remove finalizer.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Modifies managed reconciler to skip attempting to connect to external
client as well as initialize and resolve references when managed
resource has deletion timestamp and deletion policy is orphan.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Updates the APIPatchingApplicator with a description that more
accurately describes behavior when passing an object with a resource
version set.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>