Skip to content
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

Added Injected Identity Source #336

Conversation

bradkwadsworth-mw
Copy link

Added injected identity credentials source to CommonCredentialExtractor.

Signed-off-by: Brad Wadsworth [email protected]

Description of your changes

Added xpv1.CredentialsSourceInjectedIdentity as an option for the resource.CommonCredentialExtractor function. In this case it will return a nil []byte.

"Fixes #335":

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

muvaf and others added 30 commits August 31, 2021 10:41
Signed-off-by: Muvaffak Onus <[email protected]>
This commit is intended to address two issues that we diagnosed while
investigating crossplane-contrib/provider-aws#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 <[email protected]>
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 <[email protected]>
Account for two different kinds of consistency issues
We recently started using release branches for crossplane-runtime, so this will
help backport patches.

Signed-off-by: Nic Cope <[email protected]>
This is primarily for the /backport command that we can use to backport merged PRs.

Signed-off-by: Nic Cope <[email protected]>
Add commands Github workflow
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
pkg/errors#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 <[email protected]>
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 <[email protected]>
Add an `errors` package with a similar API to `github.com/pkg/errors`
I don't believe these are used anywhere anymore.

Signed-off-by: Nic Cope <[email protected]>
crossplane#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 <[email protected]>
Set `Creating` and `Deleting` conditions close to `Status().Update()` calls
https://github.com/crossplane/crossplane/tree/b7ce021e32/internal/feature
crossplane/crossplane#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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
This will report that flags aren't enabled if *Flags is nil, rather than panicing.

Signed-off-by: Nic Cope <[email protected]>
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 <[email protected]>
Add a `controller.Options` type
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 crossplane/crossplane#2595 for details.

Signed-off-by: Nic Cope <[email protected]>
…NewAPIFinalizer calls outside of package managed

Signed-off-by: Muvaffak Onus <[email protected]>
managed: make finalizer name string public
turkenh and others added 27 commits March 12, 2022 23:25
owners: add turkenh as maintainer
Signed-off-by: Muvaffak Onus <[email protected]>
Add validator and mutator chain executors to be used by provider webhooks
Signed-off-by: Sergen Yalçın <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
…rence

Support for having circular dependencies while using referencers
Added injected identity credentials source to CommonCredentialExtractor.

Signed-off-by: Brad Wadsworth <[email protected]>
@bradkwadsworth-mw bradkwadsworth-mw changed the base branch from master to release-0.15 June 22, 2022 06:30
@bradkwadsworth-mw bradkwadsworth-mw deleted the feature/injectedidentity branch June 22, 2022 06:34
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.

Support Injected Identity as a Common Credential Extractor
9 participants