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

[Q2] Refactor kubeconfig token replace logic #1165

Merged
merged 1 commit into from Jul 14, 2023

Conversation

a-blender
Copy link
Contributor

@a-blender a-blender commented Jul 5, 2023

Issue: #841

Problem

Solution

Summary of what has changed

  • Update kubeconfig token validation and replace logic in rancher2_cluster resource
  • Update/add comments
  • Update error message formatting
  • Remove unused util functions

More details

Update to kubeconfig logic to include cases where 1) the kubeconfig is empty (which happens on first provisioning of a cluster if the cache or tf.state file are manually deleted) or 2) the token is expired but returned from isKubeconfigValid as an empty string. This case will happen if the user sets a custom value for the global setting kubeconfig-default-token-ttl-minutes in Rancher.

The replace token code also needed to be updated. Before, we were generating an entire kubeconfig every time we needed a token. Instead of deleting the old token and using the one from the new kubeconfig, we are now replacing the token in the cached kubeconfig OR creating a new token for the cached kubeconfig if the token is expired or removed. If a token was removed by the user, that will force a customer to reprovision their entire TF cluster -- this case is handled gracefully now.

@kinarashah I have kept the code to check token length if the kubeconfig is invalid. This case must be isolated because if the kubeconfig can't be pulled for other reasons/errors, Terraform must download a new one. Our previous discussion made the download code unreachable.

Testing

Engineering Testing

Manual Testing

  • Provision a single node, all-role rke cluster via TF.
  • Verify the kubeconfig was downloaded correctly
  • Run terraform plan 3 times. Verify the same cached kubeconfig is being used and additional tokens were not generated
  • Set kubeconfig-default-token-ttl-minutes to 2m and run tf update to add 1 node to the cluster. Verify a token is created with 2m expiry date
  • Run tf apply to add 1 node to the cluster. Verify a new token is created before the old one expires
  • Run tf apply to add 1 node to the cluster. Verify a new token is created once the old one is removed / does not exist anymore
  • Edit terraform.tfstate file and set kube_config to a corrupt id, perhaps TESTTOKEN. Run tf apply to add 1 node to the cluster. Verify a new token is created since the old one is corrupt, no errors

All updates completed gracefully and successfully, with no errors.

Automated Testing

QA Testing Considerations

Regressions Considerations

@a-blender a-blender requested a review from kinarashah July 5, 2023 16:49
@a-blender a-blender force-pushed the fix-kubeconfig-token-logic-2 branch 6 times, most recently from 7b15f89 to 1a03476 Compare July 11, 2023 15:14
@a-blender a-blender changed the title Fix kubeconfig logic to include token expiry case Fix kubeconfig logic for expired and missing tokens Jul 11, 2023
@a-blender a-blender changed the title Fix kubeconfig logic for expired and missing tokens Fix kubeconfig logic for expired and non-existent tokens Jul 11, 2023
@a-blender a-blender requested review from a team and removed request for a team and kinarashah July 11, 2023 15:15
@a-blender a-blender force-pushed the fix-kubeconfig-token-logic-2 branch from 1a03476 to 9b04ffe Compare July 11, 2023 16:10
@a-blender a-blender changed the title Fix kubeconfig logic for expired and non-existent tokens Refactor kubeconfig logic for expired and non-existent tokens Jul 11, 2023
@a-blender a-blender requested review from jakefhyde, kinarashah and a team July 11, 2023 16:11
@a-blender a-blender changed the title Refactor kubeconfig logic for expired and non-existent tokens [Q2] Refactor kubeconfig logic for expired and non-existent tokens Jul 11, 2023
@a-blender a-blender changed the title [Q2] Refactor kubeconfig logic for expired and non-existent tokens [Q2] Refactor kubeconfig token replace logic Jul 11, 2023
@a-blender a-blender force-pushed the fix-kubeconfig-token-logic-2 branch from 9b04ffe to 12f4d18 Compare July 12, 2023 18:03
@a-blender
Copy link
Contributor Author

a-blender commented Jul 12, 2023

@kinarashah and I discussed the use of client.Token.Replace in the case of a corrupt kubeconfig token (if a user edits the TF state file manually or the token id gets corrupt by accident). This function runs a PUT action on the *Token.Token field of the Token struct, but it's probably not going to do what we think it does... because, according to the following comment

// PUT requests to Norman have trouble with nested objects due to the
    // merging strategy performed on the backend. Whenever a field is
    // removed, the resource should be replaced instead of merged,
    // and the PUT request should have a query param _replace=true.

Norman has trouble distinguishing whether an API call like that is trying to update/remove a field.

We want to update it with a new token, but Norman may misinterpret that.

Regardless, the call to get a token ByID will fail if the token id is corrupt, based on my testing. Basically, if the token id is corrupt the client won't be able to retrieve or replace it anyway so I've updated the code to use the same Create API call and create a new token to handle this case.

@a-blender a-blender requested a review from a team July 12, 2023 18:14
@a-blender a-blender force-pushed the fix-kubeconfig-token-logic-2 branch from 12f4d18 to 28eb62b Compare July 13, 2023 15:15
@a-blender a-blender requested review from kinarashah and a team July 13, 2023 15:15
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.

None yet

3 participants