Skip to content

Commit

Permalink
Merge pull request #567 from kian99/skip-requiring-ca-cert
Browse files Browse the repository at this point in the history
#567

## Description

This removes the requirement to provide a CA certificate to the provider config.
The change to require a CA cert was introduced in v0.9.0 of the provider but in certain scenarios a CA cert is not needed, e.g. 
- When a Juju controller is behind a reverse proxy signed with a trusted cert.
- When the Juju controller's CA cert is already added to the machine's cert pool.

This change is not fully backwards compatible because if a user relied on existing behaviour where partial configuration of the provider comes from the plan and the remainder is fetched via the Juju CLI, that will no longer work.

Fixes: #558, [CSS-10590](https://warthogs.atlassian.net/browse/CSS-10590)


## Type of change

- Change existing resource (change provider config)
- Bug fix (non-breaking change which fixes an issue)
- Breaking change (fix or feature that would cause existing functionality to not work as expected)

## QA steps

Manual QA steps should be done to test this PR.

1. Bootstrap controller
2. Test plan without CA specificed.
```tf
terraform {
 required_providers {
 juju = {
 version = "0.14.0"
 source = "juju/juju"
 }
 }
}

provider "juju" {
 controller_addresses = "10.16.149.83:17070"
 username = "admin"
 password = "23313145a55949e1b0fad44f5dcb78b2"
 # ca_certificate = file("./ca-cert.pem")
}

resource "juju_model" "development" {
 name = "development"
}
```
Fails with
```
$ terraform plan

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: x509: certificate signed by unknown authority
│ 
│ with provider["registry.terraform.io/juju/juju"],
│ on main.tf line 10, in provider "juju":
│ 10: provider "juju" {
│ 
│ The ca_certificate provider property is not set and the Juju certificate authority is not trusted by your system
```
3. Plan with CA cert uncommented works.
4. Plan with empty provider config works.


[CSS-10590]: https://warthogs.atlassian.net/browse/CSS-10590?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Sep 20, 2024
2 parents e96af9c + 94923c5 commit 96c412b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
10 changes: 2 additions & 8 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ func (j jujuProviderModel) valid() bool {
validClientCredentials := j.loginViaClientCredentials()

return j.ControllerAddrs.ValueString() != "" &&
j.CACert.ValueString() != "" &&
(validUserPass || validClientCredentials) &&
!(validUserPass && validClientCredentials)
}
Expand Down Expand Up @@ -316,9 +315,6 @@ func getJujuProviderModel(ctx context.Context, req provider.ConfigureRequest) (j
if planEnvVarDataModel.ControllerAddrs.ValueString() == "" {
diags.AddError("Controller address required", "The provider must know which juju controller to use. Please add to plan or use the JUJU_CONTROLLER_ADDRESSES environment variable.")
}
if planEnvVarDataModel.CACert.ValueString() == "" {
diags.AddError("Controller CACert required", "For the Juju certificate authority to be trusted by your system. Please add to plan or use the JUJU_CA_CERT environment variable.")
}
}
if diags.HasError() {
return planEnvVarDataModel, diags
Expand Down Expand Up @@ -349,9 +345,6 @@ func getJujuProviderModel(ctx context.Context, req provider.ConfigureRequest) (j
if errMsgDataModel.ControllerAddrs.ValueString() == "" {
diags.AddError("Controller address required", "The provider must know which juju controller to use.")
}
if errMsgDataModel.CACert.ValueString() == "" {
diags.AddError("Controller CACert required", "For the Juju certificate authority to be trusted by your system.")
}
if diags.HasError() {
tflog.Debug(ctx, "Current login values.",
map[string]interface{}{"jujuProviderModel": planData})
Expand Down Expand Up @@ -400,8 +393,9 @@ func checkClientErr(err error, config juju.ControllerConfiguration) diag.Diagnos
var diags diag.Diagnostics

x509error := &x509.UnknownAuthorityError{}
x509HostError := &x509.HostnameError{}
netOpError := &net.OpError{}
if errors.As(err, x509error) {
if errors.As(err, x509error) || errors.As(err, x509HostError) {
errDetail = "Verify the ca_certificate property set on the provider"

if config.CACert == "" {
Expand Down
14 changes: 14 additions & 0 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,20 @@ func TestProviderConfigurex509InvalidFromEnv(t *testing.T) {
assert.Equal(t, "x509: certificate signed by unknown authority", err.Summary())
}

func TestProviderAllowsEmptyCACert(t *testing.T) {
jujuProvider := NewJujuProvider("dev")
//Set the CA cert to be empty and check that the provider still tries to connect.
t.Setenv(JujuCACertEnvKey, "")
t.Setenv("JUJU_CA_CERT_FILE", "")
confResp := configureProvider(t, jujuProvider)
// This is a live test, expect that the client connection will fail.
assert.Equal(t, confResp.Diagnostics.HasError(), true)
err := confResp.Diagnostics.Errors()[0]
assert.Equal(t, diag.SeverityError, err.Severity())
assert.Equal(t, "The ca_certificate provider property is not set and the Juju certificate authority is not trusted by your system", err.Detail())
assert.Equal(t, "x509: certificate signed by unknown authority", err.Summary())
}

func testAccPreCheck(t *testing.T) {
if TestClient != nil {
return
Expand Down

0 comments on commit 96c412b

Please sign in to comment.