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.

Sign up for GitHub

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

Jump to bottom

[v9] tbot configure command for assisting Machine ID configuration (#12517) #12576

Merged
merged 1 commit into from
May 17, 2022
Merged

[v9] tbot configure command for assisting Machine ID configuration (#12517) #12576

merged 1 commit into from
May 17, 2022

Conversation

strideynet
Copy link
Contributor

Backport #12517 to V9

@strideynet strideynet requested a review from timothyb89 May 11, 2022 15:34
@github-actions github-actions bot requested review from LKozlowski and zmb3 May 11, 2022 15:34
@strideynet strideynet enabled auto-merge (squash) May 11, 2022 16:09
@strideynet strideynet changed the title tbot configure command for assisting Machine ID configuration (#12517) [v9] tbot configure command for assisting Machine ID configuration (#12517) May 11, 2022
@strideynet
tbot configure command for assisting Machine ID configuration (#12517)
750a5e6 
* add `tbot configure` command

* introduce golden file test helper

* address PR comments by zmb
@strideynet strideynet force-pushed the strideynet/v9/backport-pr-12517 branch from 548fae4 to 750a5e6 Compare May 17, 2022 07:58
Tener
Tener reviewed May 17, 2022
View reviewed changes
Copy link
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

It looks good but... is this not a breaking change to v9?

@strideynet
Copy link
Contributor Author

It looks good but... is this not a breaking change to v9?

I can see an argument for it being a breaking change, but I think I'd argue that:

  • The previous config command was hidden, and not advertised to customers as existing. As far as I can tell, it was just being used by Teleport engineers when developing machine ID functionality.
  • The previous config command wasn't producing output in a particularly parseable format, which reduces the likelihood further than someone was making use of it.

zmb3
zmb3 approved these changes May 17, 2022
View reviewed changes
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

No need to be concerned about breaking changes here as machine ID is only in preview for v9. LGTM

@strideynet strideynet merged commit 19beb8f into branch/v9 May 17, 2022
@strideynet strideynet deleted the strideynet/v9/backport-pr-12517 branch May 17, 2022 10:35
@webvictim webvictim mentioned this pull request Jun 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tener Tener Tener left review comments

@zmb3 zmb3 zmb3 approved these changes

@timothyb89 timothyb89 timothyb89 approved these changes

@marcoandredinis marcoandredinis marcoandredinis approved these changes

@LKozlowski LKozlowski Awaiting requested review from LKozlowski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@strideynet @Tener @marcoandredinis @timothyb89 @zmb3