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

feat: kubernetes deployment target #178

Merged
merged 9 commits into from
Dec 19, 2022
Merged

feat: kubernetes deployment target #178

merged 9 commits into from
Dec 19, 2022

Conversation

domenicsim1
Copy link
Collaborator

No description provided.

@domenicsim1 domenicsim1 requested a review from a team December 15, 2022 04:26
@benPearce1
Copy link
Contributor

there is a common deployment-target view command that attempts to defer to the specific implementations, this will need to be updated for to use the k8s view

@domenicsim1
Copy link
Collaborator Author

there is a common deployment-target view command that attempts to defer to the specific implementations, this will need to be updated for to use the k8s view

Missed that, view updated.

@domenicsim1 domenicsim1 force-pushed the feat/kubernetes-deployment-target branch from a572755 to 3fa6aed Compare December 16, 2022 02:39
benPearce1
benPearce1 reviewed Dec 16, 2022
View reviewed changes
pkg/cmd/target/kubernetes/view/view.go Outdated
data := []*output.DataRow{}
endpoint := targetEndpoint.(*machines.KubernetesEndpoint)

data = append(data, output.NewDataRow("Kubernetes Cluster URL", endpoint.ClusterURL.String()))
Copy link
Contributor

@benPearce1 benPearce1 Dec 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The cluster url property isn't used for all auth types, this one is Azure
image

The view command should probably include the auth type, and perhaps some specific detail for each type

benPearce1
benPearce1 approved these changes Dec 16, 2022
View reviewed changes
Copy link
Contributor

@benPearce1 benPearce1 left a comment

Choose a reason for hiding this comment

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

One comment on the view command, but other than that looks good

@domenicsim1 domenicsim1 force-pushed the feat/kubernetes-deployment-target branch from 33c58f6 to 4d75ddd Compare December 18, 2022 21:42
@domenicsim1 domenicsim1 merged commit 6e25759 into main Dec 19, 2022
@domenicsim1 domenicsim1 deleted the feat/kubernetes-deployment-target branch December 19, 2022 03:33
@team-integrations-fnm-bot team-integrations-fnm-bot mentioned this pull request Dec 19, 2022
Merged
This was referenced Jul 28, 2023
Closed
Merged
@team-integrations-fnm-bot team-integrations-fnm-bot mentioned this pull request Jun 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benPearce1 benPearce1 benPearce1 approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@domenicsim1 @benPearce1