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

Fix credential regression #312

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Conversation

hmlanigan
Copy link
Member

Description

Juju's CreateModel and ModelInfo provide different data for the credential used, the former returns a CloudCredential as a string, the latter returns a CloudCredentialTag as a tag. When they are both the same data. This led to different data being written as a model's credential in the terraform model resource Read and Create methods.

The second step is to reduce the juju concerns within the provider pieces of code by updating CreateModelResponse to contain what is required in the format required, rather than a struct from the juju package. This should be done more as we can.

Due to privacy concerns around cloud credentials, creating an acceptance test for this change is problematic and not done at this time. Perhaps in the future a private GitHub action and environment can be created to test.

Fixes: #310

Type of change

  • Bug fix (non-breaking change which fixes an issue)

QA steps

terraform {
  required_providers {
    juju = {
      version = ">= 0.9.0"
      source  = "juju/juju"
    }
  }
}

provider "juju" {
}

resource "juju_model" "testmodel" {
  name = "applicationtest"
}

resource "juju_model" "bugmodel" {
  name = "credentialtest"
  cloud {
    name = "aws"
    region = "us-east-1"
  }
  credential = juju_credential.test-creds.name
}

resource "juju_credential" "test-creds" {
  name = "test-creds"
  cloud  {
    name = "aws"
  }
  auth_type = "access-key"
  attributes = {
    access-key = <add-your-details>
    secret-key = <add-your-details>
  }
}

resource "juju_application" "ubuntu" {
  model = juju_model.bugmodel.name
  charm {
    name     = "ubuntu"
    series   = "jammy"
  }
}

resource "juju_application" "application_example" {
  name  = "app-example"
  model = juju_model.testmodel.name
  charm {
    name     = "ubuntu"
    series   = "jammy"
  }
}

Run with the above plan

$ juju bootstrap localhost terraform
$ juju add-cloud aws --controller terraform --force
$ terraform init && terraform plan && terraform apply
$ terraform plan && terraform apply

Additional notes

JUJU-4628

Create and Read model were setting different values for the cloud
credential used by the model, causing a failure. The create model
response data had a credential string of
<cloud>/<juju-user>/<cred-name>. ReadModel returns a credential tag
which only has the name.
Juju implementation details are leaking from the internal juju library
into the provisioner code. E.G. the provisioner does not need to now the
difference between a credential name and tag. This also reduces package
dependencies. This is one step along the way to separation of concerns.

Having the provision handle terraform and juju details, led to issue
310, where two things called CloudCredential were not equivalent.
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, QA went well 👍

Totally agree about the thicker boundary/separation with the Juju structs.

@hmlanigan hmlanigan merged commit d14194e into juju:main Sep 18, 2023
14 checks passed
@hmlanigan hmlanigan deleted the fix-credential-regression branch September 18, 2023 20:31
@hmlanigan hmlanigan added this to the 0.9.1 milestone Sep 19, 2023
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.

Credential name doesn't work since 0.9.0
2 participants