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

2.5 - Implement InstanceFirewaller for OCI Instances #10288

Merged
merged 4 commits into from
Jun 6, 2019

Conversation

manadart
Copy link
Member

@manadart manadart commented Jun 5, 2019

Description of change

This patch implements the InstanceFirewaller interface on the ociInstance type. It uses the SshInstanceConfigurator 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:

  • Bootstrap to OCI
  • 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.
  • Ensure the result is "pass".

Bug reference

N/A

@manadart manadart force-pushed the 2.5-oci-instance-firewaller branch from afb235e to edcc06a Compare June 5, 2019 13:06
Copy link
Contributor

@achilleasa achilleasa left a 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.

provider/oci/instance.go Outdated Show resolved Hide resolved
@manadart
Copy link
Member Author

manadart commented Jun 6, 2019

$$merge$$

@jujubot jujubot merged commit b00789c into juju:2.5 Jun 6, 2019
manadart added a commit to manadart/juju that referenced this pull request Jun 6, 2019
jujubot added a commit that referenced this pull request Jun 6, 2019
#10293

## Description of change

Merge 2.5 into 2.6 bringing in #10288
@manadart manadart deleted the 2.5-oci-instance-firewaller branch June 6, 2019 13:23
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants