-
Notifications
You must be signed in to change notification settings - Fork 508
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
2.5 - Implement InstanceFirewaller for OCI Instances #10288
Merged
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
…e configurator, which manipulates iptables directly.
…new test to assert correct operation.
manadart
force-pushed
the
2.5-oci-instance-firewaller
branch
from
June 5, 2019 13:06
afb235e
to
edcc06a
Compare
achilleasa
approved these changes
Jun 5, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a minor comment. Otherwise, it LGTM and QA works as described in the PR description.
|
manadart
added a commit
to manadart/juju
that referenced
this pull request
Jun 6, 2019
5 tasks
jujubot
added a commit
that referenced
this pull request
Oct 19, 2023
#16394 This patch aims to fix https://bugs.launchpad.net/juju/+bug/1834974 where we cannot apply iptables rules on OCI. This happens because on OCI (and Vsphere and Equinix) we use the `SshInstanceConfigurator` which applies iptables' rules via ssh, but on the units the authorized_keys file under /home/ubuntu/.ssh is not fully populated: the system identity public key is missing and therefore the controller machine cannot ssh onto it. We fix this by adding this missing key to the model config at model creation. ## Checklist *If an item is not applicable, use `~strikethrough~`.* - [X] Code style: imports ordered, good names, simple structure, etc - [X] Comments saying why design decisions were made - [X] Go unit tests, with comments saying what you're testing - [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~ - [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~ ## QA steps Reproduce the QA steps from this PR #10288: ``` juju bootstrap --config compartment-id=ocid1.compartment.oc1..aaaaaaaanvu2racnlczevenu73dlcf3nokgsjpdkbdgp4xbrz3lb2y4giyjq oci-canonical c juju deploy ubuntu -n 2 juju deploy 'juju-qa-network-health' juju expose network-health juju add-relation ubuntu network-health juju run --unit ubuntu/0 curl <public IP of machine 1>:8039 ``` The last command should `pass`: ``` juju run --unit ubuntu/0 curl <public IP of machine 1>:8039 pass % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 4 0 4 0 0 2000 0 --:--:-- --:--:-- --:--:-- 2000 ``` also, if you ssh onto any created machine, you should see the correct authorized keys: ``` juju ssh 0 ubuntu@juju-6ddb09-0:~/.ssh$ cat authorized_keys ssh-rsa AAAAB3NzaC1yc2... Juju:juju-client-key ssh-ed25519 AAAAC3NzaC... Juju:nicolas@thinkpad ssh-rsa AAAAB3NzaC1yc2... Juju:juju-system-key ``` ## Links **Launchpad bug:** https://bugs.launchpad.net/juju/+bug/1834974 **Jira card:** JUJU-4678
5 tasks
jujubot
added a commit
that referenced
this pull request
Feb 1, 2024
#16795 Every Juju controller has generated for it a system ssh key that allows the controller itself to ssh into machines and containers. Predominantly this has been used for configuring machine based firewalls. A fix was introduced as part of #16394 that took the juju-system-ssh key as part of the controllers model config and appended it to newly created model's authorised key. The problems this introduces is that it special cases the controller model as a defaults source for models which is not the design Juju is attempting to achieve. It also means that during model migrations we are migrating the system ssh key of one controller to another potentially introducing a security bug into the model where the previous controller could still access machines of a migrated model. With this PR we have made the bootstrap client instead generate the system ssh key and place it into the controller's config. In the API facades we now form a union of the two ssh key sources before returning them to the caller. This work also allows for the continuation of moving model config over to Dqlite as now we don't have to special case the controllers model config in the design. As part of this PR we have also updated some wording around model defaults at bootstrap time that was confusing and not correct. Newly created instance configs will also get the controllers system ssh key. ## Checklist - [x] Code style: imports ordered, good names, simple structure, etc - [x] Comments saying why design decisions were made - [x] Go unit tests, with comments saying what you're testing - [x] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing - [x] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages ## QA steps Reproduce the QA steps from this PR #10288: ``` juju bootstrap --config compartment-id=ocid1.compartment.oc1..aaaaaaaanvu2racnlczevenu73dlcf3nokgsjpdkbdgp4xbrz3lb2y4giyjq oci-canonical c juju deploy ubuntu -n 2 juju deploy 'juju-qa-network-health' juju expose network-health juju add-relation ubuntu network-health juju exec --unit ubuntu/0 curl <public IP of machine 1>:8039 ``` The last command should `pass`: ``` juju run --unit ubuntu/0 curl <public IP of machine 1>:8039 pass % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 4 0 4 0 0 2000 0 --:--:-- --:--:-- --:--:-- 2000 ``` also, if you ssh onto any created machine, you should see the correct authorized keys: ``` juju ssh 0 ubuntu@juju-6ddb09-0:~/.ssh$ cat authorized_keys ssh-rsa AAAAB3NzaC1yc2... Juju:juju-client-key ssh-ed25519 AAAAC3NzaC... Juju:nicolas@thinkpad ssh-rsa AAAAB3NzaC1yc2... Juju:juju-system-key ``` ### Test 2 We also want to make sure that the system ssh key is not settable at bootstrap time. Running: `juju bootstrap --config system-ssh-keys="..."` should result in ERROR "system-ssh-keys" is not a user configurable item. Please unset this and continue. ## Documentation changes N/A ## Links **Jira card:** JUJU-5094
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.
Description of change
This patch implements the
InstanceFirewaller
interface on theociInstance
type. It uses theSshInstanceConfigurator
to manipulate IP tables via SSH.QA steps
The
assess-network-health
functional test will pass for the OCI provider. In manual steps this is:juju deploy ubuntu -n 2
.juju deploy '~juju-qa/network-health'
.juju expose ubuntu
.juju expose network-health
.juju add-relation ubuntu network-health
.juju run --unit ubuntu/0 curl <public IP of machine 1>:8039
.Bug reference
N/A