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

Add juju_access_offer schema #627

Open
amandahla opened this issue Nov 11, 2024 · 6 comments
Open

Add juju_access_offer schema #627

amandahla opened this issue Nov 11, 2024 · 6 comments
Labels
area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement

Comments

@amandahla
Copy link
Contributor

amandahla commented Nov 11, 2024

Type of change

Adding new schema

Description

In the current version, the operator can create an offer via the Juju Terraform Provider but cannot grant or revoke permissions, meaning this must be done manually.

The juju_access_offer resource aims to provide the same functionality as the existing `juju_jaas_access_offer` resource, where the operator can set users and access levels for an offer.

Details

"access": schema.StringAttribute{
	Description: "Level of access to grant. Changing this value will replace the Terraform resource. Valid access levels are described at https://juju.is/docs/juju/manage-offers#control-access-to-an-offer",
	Required:    true,
	PlanModifiers: []planmodifier.String{
		stringplanmodifier.RequiresReplace(),
	},
        Validators: []validator.Set{ setvalidator.ValueStringsAre(validator.OneOf("read", "consume", "admin"),),},
},
"users": schema.SetAttribute{
	Description: "List of users to grant access.",
	Optional:    true,
	ElementType: types.StringType,
	Validators: []validator.Set{
		setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")),
	},
},
// ID required for imports
"id": schema.StringAttribute{
	Computed: true,
	PlanModifiers: []planmodifier.String{
		stringplanmodifier.UseStateForUnknown(),
	},
},
"offer_url": schema.StringAttribute{
	Description: "The url of the offer for access management. If this is changed the resource will be deleted and a new resource will be created.",
	Required:    true,
	Validators: []validator.String{
		ValidatorMatchString(func(s string) bool {
			_, err := crossmodel.ParseOfferURL(s)
			return err == nil
		}, "offer_url must be a valid offer string."),
	},
	PlanModifiers: []planmodifier.String{
		stringplanmodifier.RequiresReplace(),
	},
}

Example terraform plan

resource "juju_access_offer" "development" {
  offer_url        = juju_offer.myoffer.url
  access           = "consume"
  users            = ["foo", "bar"]
}

Notes & References

  • This schema is a step for providing CMR via Juju Terraform Provider. It was discussed in the last sprint (Oct/2024).

  • When removing/changing a juju_access_offer we should have in mind that revoke does not remove all access levels as per the documentation example:

To grant bob consume access to an offer:

juju grant bob consume admin/default.hosted-mysql

To revoke bob’s consume access (he will be left with read access):

juju revoke bob consume admin/default.hosted-mysql

To revoke all of bob’s access:

juju revoke bob read admin/default.hosted-mysql
@amandahla amandahla added area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement state/untriaged untriaged bug report labels Nov 11, 2024
@hmlanigan hmlanigan removed the state/untriaged untriaged bug report label Nov 12, 2024
@hmlanigan
Copy link
Member

@amandahla,

Overall this look good, thank you for the detail.

I'm unsure of stringplanmodifier.RequiresReplace() for changing the access level. Why do the extra work? It's done that way for JaaS due to limitation of their API as far as I know.

I just noticed that the juju_access_model resource does the same thing which has me scratching my head. The rest of the code has some logic errors in this area too.

I'd prefer to have improved behavior rather than repeating mistakes made in juju_access_model.

@amandahla
Copy link
Contributor Author

In that case, the resource would need to check during the update whether a grant or revoke should be run, right? For example, if the access level is initially 'admin' and then changes to 'read', the provider should call 'revoke' instead of 'grant'.

@amandahla
Copy link
Contributor Author

Hi, although the issue was not closed, I started working on a draft PR that is now ready for review. While working on it, I noticed why Access was set to RequiresReplace: it changes the ID, and without it, the update fails. I’ve marked the PR as ready for review, but if there are any updates on this issue, I can revert it back to a draft.

@amandahla
Copy link
Contributor Author

Considering what was discussed in the last terraform office hours, this is a proposal for a new schema:

Thanks @alesstimec for the idea.

resp.Schema = schema.Schema{
		Description: "A resource that represent a Juju Access Offer. Warning: Do not repeat users across different access levels.",
		Attributes: map[string]schema.Attribute{
			string(permission.AdminAccess): schema.SetAttribute{
				Description: "List of users to grant admin access.",
				Optional:    true,
				ElementType: types.StringType,
				Validators: []validator.Set{
					setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")),
				},
			},
			string(permission.ConsumeAccess): schema.SetAttribute{
				Description: "List of users to grant consume access.",
				Optional:    true,
				ElementType: types.StringType,
				Validators: []validator.Set{
					setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")),
				},
			},
			string(permission.ReadAccess): schema.SetAttribute{
				Description: "List of users to grant read access.",
				Optional:    true,
				ElementType: types.StringType,
				Validators: []validator.Set{
					setvalidator.ValueStringsAre(ValidatorMatchString(names.IsValidUser, "user must be a valid Juju username")),
				},
			},
			// ID required for imports
			"id": schema.StringAttribute{
				Computed: true,
				PlanModifiers: []planmodifier.String{
					stringplanmodifier.UseStateForUnknown(),
				},
			},
			"offer_url": schema.StringAttribute{
				Description: "The url of the offer for access management. If this is changed the resource will be deleted and a new resource will be created.",
				Required:    true,
				Validators: []validator.String{
					ValidatorMatchString(func(s string) bool {
						_, err := crossmodel.ParseOfferURL(s)
						return err == nil
					}, "offer_url must be a valid offer string."),
				},
				PlanModifiers: []planmodifier.String{
					stringplanmodifier.RequiresReplace(),
				},
			},
		},
	}

@alesstimec
Copy link
Member

this look ok to me, but is it possible to validate that a specific user only appears in one of the sets?

@amandahla
Copy link
Contributor Author

amandahla commented Dec 13, 2024

During the creation I thought about adding a validation for it like this:
https://github.com/juju/terraform-provider-juju/pull/633/files#diff-19abca65ce9f20c12ad421456eb38074560ed37ab200820304ff1114794bffe0R151

Actually I can write a validateconfig for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Review schema changes before work begins kind/feature suggests new feature or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants