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

NET-5879 - consul-template support for sameness groups #1899

Merged
merged 16 commits into from
Jun 18, 2024
Merged

NET-5879 - consul-template support for sameness groups #1899

merged 16 commits into from
Jun 18, 2024

Conversation

jmurret
Copy link
Member

@jmurret jmurret commented Mar 19, 2024 •
edited
Loading

consul 1.19 will allow passing the sameness-group param to the /health endpoints. This PR allows consul template to utilize that functionality.

jmurret
jmurret commented Apr 24, 2024
View reviewed changes
.golangci.yml Show resolved Hide resolved
jmurret
jmurret commented Apr 24, 2024
View reviewed changes
dependency/vault_pki.go Outdated Show resolved Hide resolved
@jmurret jmurret force-pushed the jm/NET-5879 branch 2 times, most recently from b800110 to 5736d63 Compare June 3, 2024 16:44
@jmurret jmurret marked this pull request as ready for review June 3, 2024 17:27
@jmurret jmurret requested review from jm96441n, lkysow and nathancoleman and removed request for lkysow June 3, 2024 17:37
lkysow
lkysow reviewed Jun 3, 2024
View reviewed changes
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Looks great. Only thing "blocking" from me is the question about other things to test.

dependency/dependency_test.go Outdated Show resolved Hide resolved
dependency/dependency_test.go Outdated Show resolved Hide resolved
dependency/dependency_test.go Outdated Show resolved Hide resolved
dependency/health_service.go Outdated Show resolved Hide resolved
dependency/health_service_test.go Show resolved Hide resolved
dependency/health_service_test.go Show resolved Hide resolved
dependency/dependency_test.go Show resolved Hide resolved
dependency/dependency_test.go Outdated Show resolved Hide resolved
dependency/health_service_test.go Outdated Show resolved Hide resolved
dependency/health_service_test.go Show resolved Hide resolved
lkysow
lkysow reviewed Jun 3, 2024
View reviewed changes
dependency/health_service.go Outdated Show resolved Hide resolved
dependency/health_service.go Outdated Show resolved Hide resolved
dependency/dependency.go Outdated Show resolved Hide resolved
jmurret and others added 14 commits June 18, 2024 13:17
@jmurret
NET-5879 - consul-template support for sameness groups
d5fcee2
@jmurret
Get TestHealthServiceQuery_Fetch_SamenessGroup working
3c320c1
@jmurret
add SamenessGroup to Merge()
b182e68
@jmurret
update deprecated golangci setting
cf00e10
@jmurret
fix linting error
f9ff0d8
@jmurret
fixing failing test
1317ef7
@jmurret
add blocking query tests for sameness groups.
31448b4
@jmurret
add docs
64b76c0
@jmurret @lkysow
Apply suggestions from code review
9f3c138 
Co-authored-by: Luke Kysow <[email protected]>
@jmurret
fixing PR suggestions.
c9ea6a9
@jmurret
adding sameness-group to validation for query params for service calls
8356636
@jmurret @lkysow
Update dependency/health_service.go
897d087 
Co-authored-by: Luke Kysow <[email protected]>
@jmurret
change @samenessGroup to @sameness-group
efa1f0d
@jmurret
updated docstring for TestHealthServiceQuery_Fetch_SamenessGroup to n…
15dd6c0 
…ote that full behavior for sameness groups is asserted in the consul code base.
@jmurret jmurret force-pushed the jm/NET-5879 branch 2 times, most recently from e403b23 to 356338d Compare June 18, 2024 19:25
@jmurret jmurret merged commit 93de881 into main Jun 18, 2024
54 checks passed
@jmurret jmurret deleted the jm/NET-5879 branch June 18, 2024 20:16
nathancoleman
nathancoleman reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@nathancoleman nathancoleman left a comment

Choose a reason for hiding this comment

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

LGTM

@renovate renovate bot mentioned this pull request Jun 21, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathancoleman nathancoleman nathancoleman left review comments

@lkysow lkysow lkysow approved these changes

@jm96441n jm96441n Awaiting requested review from jm96441n

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

Successfully merging this pull request may close these issues.
3 participants
@jmurret @lkysow @nathancoleman