Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Ensure kube_resources always have the correct defaults, even after version change #1043

Merged

Conversation

hugoShaka
Copy link
Contributor

@hugoShaka hugoShaka commented Mar 21, 2024

image

This PR is required because relaxing the RequireReplace() causes the Kubernetes resource defaults bug to reappear.

For context:

  • the kubernetes_resource field can be empty for the user
  • the server and provider will set defaults
  • TF will get mad if it's not aware that this can happen, we can tell TF by marking the field as "computed"
  • When a field is computed, TF will store its result back into the state
  • The state is then used for future reconciliations
  • If you happen to change the role version, the defaults should change, but TF will not be aware and use the old defaults from the state
  • You will have a new role with older defaults, causing inconsistencies and potentially granting too much access

We tried to solve this issue by marking the resource for replacement on version change, but this proved to be super disruptive. Not all resources can be deleted easily. AccessLists contain information about its members, and roles might have dependencies (you cannot delete a role if a user or an AL is referencing it, SSO are dynamic and we cannot remove them with TF).

This PR proposes a more advanced solution. Instead of using the TF state, we re-compute the defaults based on the configuration provided in the TF code. This is done with a Plan modifier.

Copy link
Contributor

@marcoandredinis marcoandredinis left a comment

Choose a reason for hiding this comment

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

Can we please add tests?

return DefaultRoleKubeModifierDescription
}

func (d DefaultRoleKubeResourceModifier) Modify(ctx context.Context, req tfsdk.ModifyAttributePlanRequest, resp *tfsdk.ModifyAttributePlanResponse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add godoc at least for this one?

@hugoShaka
Copy link
Contributor Author

Can we please add tests?

This is tested by TestTerraform/TestRoleVersionUpgrade. This is the reason why #1042 tests are failing and cannot be merged: https://github.com/gravitational/teleport-plugins/actions/runs/8365719432/job/22904223261?pr=1042#step:6:379

@hugoShaka hugoShaka merged commit bc4fb04 into hugo/relax-version-condition Mar 25, 2024
7 checks passed
@hugoShaka hugoShaka deleted the hugo/set-correct-kuberesource-defaults branch March 25, 2024 21:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants