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

Add authorisation support to console CRDs #2

Merged
merged 191 commits into from
Sep 30, 2020
Merged

Conversation

waltfy
Copy link

@waltfy waltfy commented Aug 13, 2020

Adds fields to the ConsoleTemplate CRD that facilitates implementing
peer-reviewed consoles.

authorisationRules provides a list of rules that commands can be
matched against and if a match is found, the console requires
authorisation to run from a defined number of subjects. It is intended
that the rules will be matched from top to bottom and the first match is
used.

defaultAuthorisationRule provides a default rule to match on if no
other rules match.

Both these fields are optional allowing for backward compatibility.

An additional CRD has also been added to facilitate the authorisation
process. The ConsoleAuthorisation kind provides a way to monitor
approvals provided to a console that matches a rule, or the default
rule.

The actual implementation of the peer-reviewed console feature is to
follow.

dyson and others added 30 commits April 7, 2020 09:57
Adds fields to the ConsoleTemplate CRD that facilitates implementing
peer-reviewed consoles.

`authorisationRules` provides a list of rules that commands can be
matched against and if a match is found, the console requires
authorisation to run from a defined number of subjects. It is intended
that the rules will be matched from top to bottom and the first match is
used.

`defaultAuthorisationRule` provides a default rule to match on if no
other rules match.

Both these fields are optional allowing for backward compatibility.

An additional CRD has also been added to facilitate the authorisation
process. The `ConsoleAuthorisation` kind provides a way to monitor
approvals provided to a console that matches a rule, or the default
rule.

The actual implementation of the peer-reviewed console feature is to
follow.
…tion-console-crds

Add authorisation support to console CRDs
In the latest release of `kind`, the `kind get kubeconfig-path`
subcommand has been removed. See PR #1143 in the kind repo for further
info.

This has been replaced, to some extent, with `kind export kubeconfig`,
which sets the context to the kind cluster, and `kind get kubeconfig`:
but this dumps the whole kubeconfig to stdout, not just the path.

It is still possible to use a separate `KUBECONFIG` file, by having this
variable exported when creating the kind cluster, but there should be no
issue with just having the context added to the main `KUBECONFIG` file,
and this seems to be what the project is suggesting.

Therefore just change the invocation of commands, when we shell out, to
use an explicit `--context`, rather than overriding `KUBECONFIG`.
This change introduces a theatre-consoles kingpin application.
The application has a single command, ``create`` which will create a
console instance, ready to attach to.

This is part of a series of changes to provide a command line to
work with consoles in theatre. The management logic currently in the
consoles application will likely move to be closer to the runner in
the near future.
This change introduces a ``namespace`` and a ``context`` flag to allow
the user to specify the relevant namespace and/or kubernetes context
for the invocation of the command.
This solves the issue of running a console when multiple templates
match the selectors across namespaces, in addition to making it more
explicit and easier to include in a script.
When developing this application we are constantly invalidating the
first layer of the container, by changing the source files. This causes
us to have to rebuild `envconsul`, which takes up to a minute.

Fix this by building `envconsul` first, so that we'll keep this cached
and only need to repeat this step if changing the envconsul version.
Bumps ubuntu from bionic-20191202 to bionic-20200311.

Signed-off-by: dependabot-preview[bot] <[email protected]>
Adds a validating webhook for console authorisation updates to ensure that:

- Only one authorisation is added
- No authorisations are removed
- The added authoriser is the user making the update
- The added authoriser is not the owner of the console
- The `consoleRef` and `owner` fields aren't modified

Co-authored-by: Ben Wheatley <[email protected]>
…risation-admission

Add validating console authorisation webhook
While adding new specs to this suite it became clear that it's not laid
out ideally.

The primary change here is to split out configuration and creation of
resources, as per the style [suggested in the Ginkgo docs][0].

What this means is that it's now possible to override parts of the
console template or console within each `Describe` block, leading to
much cleaner code as the specs themselves now no longer need to create
resources before asserting against them.

Due to the introduction of some more nesting, it may be easier to review
this diff without whitespace changes.

[0]: https://onsi.github.io/ginkgo/#separating-creation-and-configuration-justbeforeeach
…ration-cleanup

Clean up console integration test
This clears up the package namespace, and prevents a conflict that will
occur in a future commit!
We discovered, while writing reconciliation code, that if you were to
perform an update against this resource but not add any authorisers,
then the update would be rejected.

This is unintuitive, it should be possible to perform operations such
as adding annotations or altering non-core fields, without any issue.

Change the validation code to only throw an error if there's greater
than 1 addition, as opposed to 0 or >1 additions.
When a console is created, that is based off a template which includes
authorisation rules, stamp out a `consoleauthorisation` object that will
be used by approvers to authorise the console instance.

This commit adds the reconciliation around the authorisation objects,
and skeleton tests, but leaves the implementation of determining whether
a console is authorised for a subsequent PR.
Add reconciliation skeleton for console authorisation
This change fixes ``make build-all``. The darwin binaries were missed
during a previous change that fixed building commands with multiple
files in the package.
This change separates the ``create`` command from the flags that drive it.
This allows for easier integration within other projects, and sets a standard for other commands to be added.
…reate-command

Refactor consoles create to separate the functionality from flags
This change inlines the runner creation, exposing the clientset and
config, which are anticipated to be required in subsequent changesets.

There are no functional changes.
This changeset corrects the passing of all arguments to the
create command.
…params

Pass all command arguments to create command
This adds the ability to attach to a console that has been created using
the previously added `create` command.
rnaveiras and others added 27 commits July 27, 2020 16:36
This annotation is not longer in use with certmanager
…d-kustomize

v2: Removed deprecated annotation
Only run the webhook when the namespace has the necessary label. It
prevents running the mutating webhook for critical namespaces like
kube-system and theatre-system so the system can recover
automatically if the workload controller goes down.

Without this changed a simple roll out of the controller manager for
workloads can bring the system down. The new controller workload pod
will invoke the pod priority webhook, but without the controller to
respond to the apiserver call, the system will grind to a halt, stoping
pod creation.
Enabled again the release workflow
…friendly

v2: kustomize updated port names
…ircleci

Updated circleci - release workflow
…mutating-webhook

v2: Updated priority-injector MutatingWebhookConfiguration
- Bump golang to 1.14.5
- Bump ubuntu to focal-20200723 (LTS)
When using zap Error logging it adds stacktrace output in an undesirable
way. For now, lets stick to using Info level logs only like we did
previously.

Given we are only using Info level logging we can for now remove some of
the additional functions supporting Error and V logs from recutils for
logs with the event recorder.

A few other minor changes in here includes adding back the componet key
value to controller logs, and removing a debug log line no longer
needed.
Update PROJECT to kube-builder v2 format with all of our CRDS.
Changed EncodeTime and enabled Caller
This package is no longer need it. The funcionality was replaced by
envtest.

See sigs.k8s.io/controller-runtime/pkg/envtest
Shuffle around the READMEs to their new locations and update any links.

A more extensive review of the READMEs (given the large structural
changes to the codebase) can come later.
…g-integration

v2: Removed pkg/integration
@waltfy waltfy merged commit 8e3ad8a into master Sep 30, 2020
@waltfy waltfy deleted the waltfy/update-from-upstream branch September 30, 2020 16:16
waltfy pushed a commit that referenced this pull request Nov 23, 2022
The current behaviour for attachment is to wait in a loop for console
pods to become ready, and then try to create the attachment. This leads
to two potential problems if the command passed to the console is very
short lived:

1. The console may be ready for such a short time that by the time we
get notified it has already stopped
2. The console runs to completion before we can successfully create an
attachment.

In both of these cases, theatre will error out, but it is not
necessarily an error: the command may merely have completed
successfully. This matters a great deal for Dispatcher console creation,
since if (to take a common case) we successfully run database migrations
for some app too quick for theatre, then we do not want to fail the
pipeline.

The solution in this commit is to simply remove the check for a stopped
pod in the wait-until-ready loop altogether (it only matters for
attachment) and then handle the attachment error #2 such that we write
the logs to the attachment out stream and exit with the appropriate
status (so we're still red if the console failed ... )
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.

8 participants