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

cli: Add consul intention list command (based on PR #6825) #9468

Merged
merged 8 commits into from
Jan 12, 2021
Merged

cli: Add consul intention list command (based on PR #6825) #9468

merged 8 commits into from
Jan 12, 2021

Conversation

karras
Copy link
Contributor

@karras karras commented Dec 24, 2020

This PR is based on the previous work by @snuggie12 in PR #6825. It adds the command consul intention list to list all available intentions. The list functionality for intentions seems a bit overdue as it's just very handy. The web UI cannot list intentions outside of the default namespace, and using the API is sometimes not the friendliest option. ;)

I cherry picked snuggie12's commits who did most of the heavy lifting (thanks again @snuggie12 for your great work!). The changes in the original commit mostly still worked on the current HEAD. On top of that I added support for namespaces and fixed the docs as they are managed differently today. Also the requested changes related to the "Connect" references in the original PRs have been addressed.

Fixes #5652

@karras karras changed the title Add consul intention list command (based on PR #6825) cli: Add consul intention list command (based on PR #6825) Dec 24, 2020
@karras
Copy link
Contributor Author

karras commented Jan 8, 2021

Please let me know if I have to do something to make the Pull Request Labeler / triage (pull_request) test green. I assume you have to assign some labels?

Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM

}

result := make([]string, 0, len(ixns))
header := "ID|Source|Action|Destination|Precedence"
Copy link
Member

Choose a reason for hiding this comment

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

Please use \x1f as the delimiter. See #6652 for more details.

Copy link
Contributor Author

@karras karras Jan 11, 2021

Choose a reason for hiding this comment

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

Done! I've quickly tested it in my Enterprise environment and it seems to look fine but may I ask you to do a short cross check?

Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM, except the delimiter.

@hanshasselberg
Copy link
Member

One more thing: could you add a file in .changelog for the changelog generation? That would be great! You can check the existing files for help, this should be an improvement. 🙇

@karras
Copy link
Contributor Author

karras commented Jan 11, 2021

One more thing: could you add a file in .changelog for the changelog generation? That would be great! You can check the existing files for help, this should be an improvement. bow

Done, let me know if it needs to be adjusted. :)

Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

Thank you for the changes!

@hanshasselberg hanshasselberg merged commit 3c3b672 into hashicorp:master Jan 12, 2021
@hashicorp-ci
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/309042.

@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 3c3b672 onto stable-website succeeded!

hashicorp-ci pushed a commit that referenced this pull request Jan 12, 2021
This PR is based on the previous work by @snuggie12 in PR #6825. It adds the command consul intention list to list all available intentions. The list functionality for intentions seems a bit overdue as it's just very handy. The web UI cannot list intentions outside of the default namespace, and using the API is sometimes not the friendliest option. ;)

I cherry picked snuggie12's commits who did most of the heavy lifting (thanks again @snuggie12 for your great work!). The changes in the original commit mostly still worked on the current HEAD. On top of that I added support for namespaces and fixed the docs as they are managed differently today. Also the requested changes related to the "Connect" references in the original PRs have been addressed.

Fixes #5652

Co-authored-by: Matt Hoey <mhoey05@jcu.edu>
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 3c3b672 onto release/1.9.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Jan 12, 2021
This PR is based on the previous work by @snuggie12 in PR #6825. It adds the command consul intention list to list all available intentions. The list functionality for intentions seems a bit overdue as it's just very handy. The web UI cannot list intentions outside of the default namespace, and using the API is sometimes not the friendliest option. ;)

I cherry picked snuggie12's commits who did most of the heavy lifting (thanks again @snuggie12 for your great work!). The changes in the original commit mostly still worked on the current HEAD. On top of that I added support for namespaces and fixed the docs as they are managed differently today. Also the requested changes related to the "Connect" references in the original PRs have been addressed.

Fixes #5652

Co-authored-by: Matt Hoey <mhoey05@jcu.edu>
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.

Implement 'consul intention list' command
4 participants