forked from gocardless/theatre
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`.
acceptance: Support kind v0.7.0
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.
Create CLI for creating consoles
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.
Reorder Dockerfile for quicker builds
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.
Fix build-all step
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.
…mand Consoles attach command
…g-apis v2: remove pkg/apis
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)
…ckerfile Updated dockerfile
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.
v2: Update PROJECT
Changed EncodeTime and enabled Caller
…-logging v2: Improve logging
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
v2: Update READMEs
Update theatre to kubebuilder v2
…d-name Fix CI manifest command name
…se-v2.0.0 v2.0.0
Fixup console doc links
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds fields to the ConsoleTemplate CRD that facilitates implementing
peer-reviewed consoles.
authorisationRules
provides a list of rules that commands can bematched 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 noother 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 monitorapprovals provided to a console that matches a rule, or the default
rule.
The actual implementation of the peer-reviewed console feature is to
follow.