Skip to content

[vercel_team_config] Fix saml dsync for access groups #298

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kitfoster
Copy link
Collaborator

At the moment, the vercel_team_config resource incorrectly implements Access Group support. If a team has SAML roles mapped access groups, the provider deletes the mappings.

This PR fixes the access group mappings for SAML Roles.

The change is breaking 👇 @dglsparsons @jarneson let me know what you think is the best way forward with the breaking changes. Would it be sufficient to rename roles?

// before
{
	"roles": schema.MapAttribute{
		Description: "Directory groups to role or access group mappings.",
		Computed:    true,
		ElementType: types.StringType,
	},
	"access_group_id": schema.StringAttribute{
		Description: "The ID of the access group to use for the team.",
		Computed:    true,
	}
}
// after
{
	"roles": schema.MapAttribute{
		Description: "Directory groups to role or access group mappings. For each directory key, either a role or access group id is specified. The role is one of 'MEMBER', 'OWNER', 'VIEWER', 'DEVELOPER', 'BILLING' or 'CONTRIBUTOR'. The access group id is the id of an access group.",
		Computed:    true,
		ElementType: types.ObjectType{
			AttrTypes: map[string]attr.Type{
				"role":            types.StringType,
				"access_group_id": types.StringType,
			},
		},
	},
}

@kitfoster kitfoster changed the title Fix saml access groups [vercel_team_config] Fix saml dsync for access groups Apr 14, 2025
),
Computed: true,
ElementType: types.ObjectType{
AttrTypes: map[string]attr.Type{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would like to add a validator that exactly one of role or access_group_id is present, but couldn't make it work

Copy link
Member

Choose a reason for hiding this comment

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

i think this just ends up being a marshalling error when we go to hit the RPCs? we could explicitly check it, not sure if terraform provides a hook for that. doug knows way more than I do on this stuff.

Enforced bool `json:"enforced"`
Roles map[string]string `json:"roles"`
Enforced bool `json:"enforced"`
Roles map[string]any `json:"roles"`
Copy link
Member

Choose a reason for hiding this comment

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

any is pretty gnarly here, when we just need to support marshaling, a union type is only a few more lines of code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice, how do I do the union type?

Copy link
Member

Choose a reason for hiding this comment

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

something like:

type UpdatSamlConfigRole struct {
  Role *string
  AccessGroupID *string
}

func (r *UpdateSamlConfigRole) MarshalJSON() ([]byte, error) {
  if r.Role != nil { return json.Marshal(*r.Role) }
  if r.AccessGroupID != nil { return json.Marshal(*r.AccessGroupID) }
  return nil, fmt.Errorf("bad union")
}

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.

2 participants