feat: add workspace_sharing attribute to coderd_organization resource #295
Conversation
- Also adds it to the coderd_organization data source - Also adds an option to the test harness to test against features gated by experiments (enabled via the CODER_EXPERIMENTS env var) Related to: coder/internal#1245
aslilac
left a comment
aslilac
left a comment
There was a problem hiding this comment.
one small thing but otherwise looks great!
| MarkdownDescription: "Workspace sharing setting for the organization. " + | ||
| "Valid values are `everyone` and `none`.", | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
| Computed: true, |
I don't think this is computed on the resource. there's no code to read this value, just to update it.
There was a problem hiding this comment.
We do read it back and set in state in Read/Create/Update. Since the backend always has a value (default
even if unset), we mark it as computed so state reflects the server state when users don’t configure it.
Is this the wrong approach?
There was a problem hiding this comment.
Since the backend always has a value (default even if unset), we mark it as computed so state reflects the server state when users don’t configure it.
Yep, this is correct. It's the same deal for a lot of the optional attributes in the template resource.
A good example ismax_port_share_level, another enum.
| workspaceSharingSettings, err := r.Client.WorkspaceSharingSettings(ctx, orgID.String()) | ||
| if err != nil { | ||
| resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to get workspace sharing settings, got error: %s", err)) | ||
| return | ||
| } |
There was a problem hiding this comment.
It looks like we're calling this new workspace sharing route unconditionally - won't this break the resource on Coder deployments that don't have the experiment enabled?
Even once workspace sharing is no longer an experiment, we still need to support Coder deployments running (I think up to 3?) older versions.
There was a problem hiding this comment.
It looks like we're calling this new workspace sharing route unconditionally - won't this break the resource on Coder deployments that don't have the experiment enabled?
I wasn't sure how we handle situations like this, so I planned to only merge it once workspace sharing is no longer an experiment. Was this the wrong approach?
Even once workspace sharing is no longer an experiment, we still need to support Coder deployments running (I think up to 3?) older versions.
Great catch, I didn't know about this!
There was a problem hiding this comment.
I wasn't sure how we handle situations like this,
Yeah, unfortunately I don't think we have any existing code that's conditionally run depending on the Coder version, just some resources that we say in the docs aren't usable unless your Coder deployment is recent enough.
We do have entitlement checks for some resources though (TemplateResourceModel.CheckEntitlements()), so you might want to do something like that.
Off the top of my head I think we'd be fine to just always try and hit the route, not report an error if it's a 404, and set the attribute to a known null when that happens?
3 participants
This PR also:
coderd_organizationdata sourceslogdependency (cdr.dev/slog->cdr.dev/slog/v3)NOTE: Draft PR: depends on unreleased Coder changes, so this uses a commit‑pinned
github.com/coder/coder/v2version until the next tag.Related to: coder/internal#1245