Deep service insight endpoint #3261

haywoodsh · 2022-11-16T13:18:35Z

Proposed changes

Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue here in this description (not in the title of the PR).

Checklist

Before creating a PR, run through this checklist and mark each as complete.

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have checked that all unit tests pass after adding my changes
I have updated necessary documentation
I have rebased my branch onto main
I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

codecov-commenter · 2022-11-16T13:25:27Z

Codecov Report

Merging #3261 (8ba1029) into main (363e697) will decrease coverage by 0.11%.
The diff coverage is 43.63%.

@@            Coverage Diff             @@
##             main    #3261      +/-   ##
==========================================
- Coverage   51.67%   51.56%   -0.12%     
==========================================
  Files          59       60       +1     
  Lines       16520    16678     +158     
==========================================
+ Hits         8537     8600      +63     
- Misses       7697     7788      +91     
- Partials      286      290       +4

Impacted Files	Coverage Δ
cmd/nginx-ingress/flags.go	`29.85% <0.00%> (-1.18%)`	⬇️
cmd/nginx-ingress/main.go	`0.00% <0.00%> (ø)`
internal/configs/configurator.go	`35.16% <0.00%> (-0.85%)`	⬇️
internal/healthcheck/healthcheck.go	`61.90% <61.90%> (ø)`
internal/configs/virtualserver.go	`95.22% <100.00%> (ø)`
...ternal/k8s/appprotect/app_protect_configuration.go	`86.16% <0.00%> (-0.58%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

internal/healthcheck/healthcheck.go

for more information, see https://pre-commit.ci

ADubhlaoich

Generally grand, just made some suggestions around capitalisation and a few missed typos!

docs/content/configuration/global-configuration/command-line-arguments.md

docs/content/logging-and-monitoring/service-insight.md

docs/content/configuration/global-configuration/command-line-arguments.md

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

deployments/helm-chart/values.yaml

Co-authored-by: Ciara Stacke <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

ciarams87 · 2022-12-02T14:51:09Z

Looks good, just noticed one or two things. The Helm documentation needs to be updated as well with the new flags

https://github.com/nginxinc/kubernetes-ingress/blob/main/deployments/helm-chart/README.md and

https://github.com/nginxinc/kubernetes-ingress/blob/main/docs/content/installation/installation-with-helm.md

ciarams87

👍🏼 🎉

github-actions bot added dependencies Pull requests that update a dependency file enhancement Pull requests for new features/feature enhancements labels Nov 16, 2022

github-advanced-security bot found potential problems Nov 16, 2022

View reviewed changes

internal/healthcheck/healthcheck.go Fixed Show fixed Hide fixed

internal/healthcheck/healthcheck.go Fixed Show fixed Hide fixed

haywoodsh and others added 22 commits November 16, 2022 14:17

poc for healthcheck inside NIC

c2ed0e6

add listener.go

b759a84

Add changes discussed in the meeting

ea63d10

Add initial implementation of the server

3405d0a

Minor syntax update

e0bebd0

Add a web server and a router

852e577

Refactor, handle err, simplify

482a673

Updated docs

b7257d1

Update data type expected by log message

dc5050c

Add TLS support for the healthcheck server

0877b5a

Fix typos

b2f1b91

Add placeholders for server tests

f08179c

Check if hostname exist and handle http 404

2c0fc9a

Update url path to include 'probe'

fa830da

Refactor handler, add tests

ae81944

create flags for health probe

8f56d3f

Handle errors from starting servers

65eff0b

Support min TLS v1.0

1938451

Update min TLS version to 1.2

b9e2d5f

Simplify package structure

3ee5480

Remove double import

dcebe1c

Round flow to the left

6ba4114

haywoodsh force-pushed the feat/deep-service-insight-endpoint branch from 55baec4 to 6ba4114 Compare November 16, 2022 14:18

jjngx added 2 commits November 16, 2022 15:19

Sanitize hostname param

d4fa8fb

Use different var name for sanitized input

9c01b84

github-advanced-security bot found potential problems Nov 16, 2022

View reviewed changes

internal/healthcheck/healthcheck.go Fixed Show fixed Hide fixed

internal/healthcheck/healthcheck.go Fixed Show fixed Hide fixed

vepatel and others added 3 commits November 30, 2022 13:23

add test for replica change

ab945e8

[pre-commit.ci] auto fixes from pre-commit.com hooks

6c04db4

for more information, see https://pre-commit.ci

Merge branch 'main' into feat/deep-service-insight-endpoint

34b9a8b

ADubhlaoich approved these changes Nov 30, 2022

View reviewed changes

jjngx and others added 12 commits December 1, 2022 09:54

Update docs/content/logging-and-monitoring/service-insight.md

2387e1d

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/configuration/global-configuration/command-line-a…

1a449fc

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

167fbfc

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

923ccb1

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

46ac908

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

9d08f94

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

ee0abb0

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/configuration/global-configuration/command-line-a…

bd25635

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/configuration/global-configuration/command-line-a…

0aafb41

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/configuration/global-configuration/command-line-a…

2075150

…rguments.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

86903f4

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

Update docs/content/logging-and-monitoring/service-insight.md

faf8245

Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

jjngx requested a review from a team December 1, 2022 10:13

Increase wait time in integration test

aec41bc

vepatel approved these changes Dec 2, 2022

View reviewed changes

shaun-nx approved these changes Dec 2, 2022

View reviewed changes

ciarams87 reviewed Dec 2, 2022

View reviewed changes

deployments/helm-chart/values.yaml Outdated Show resolved Hide resolved

Update deployments/helm-chart/values.yaml

cbf43e6

Co-authored-by: Ciara Stacke <[email protected]> Signed-off-by: Jakub Jarosz <[email protected]>

vepatel and others added 3 commits December 2, 2022 16:39

add conditional check on waitTime for api response

8ba1029

Add docs for Helm installation

48b3d72

Merge branch 'main' into feat/deep-service-insight-endpoint

1ecc63e

ciarams87 approved these changes Dec 5, 2022

View reviewed changes

jjngx merged commit 9d6f746 into main Dec 5, 2022

jjngx deleted the feat/deep-service-insight-endpoint branch December 5, 2022 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deep service insight endpoint #3261

Deep service insight endpoint #3261

haywoodsh commented Nov 16, 2022

codecov-commenter commented Nov 16, 2022 •

edited

Loading

ADubhlaoich left a comment

ciarams87 commented Dec 2, 2022

ciarams87 left a comment

Deep service insight endpoint #3261

Deep service insight endpoint #3261

Conversation

haywoodsh commented Nov 16, 2022

Proposed changes

Checklist

codecov-commenter commented Nov 16, 2022 • edited Loading

Codecov Report

ADubhlaoich left a comment

Choose a reason for hiding this comment

ciarams87 commented Dec 2, 2022

ciarams87 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 16, 2022 •

edited

Loading