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

Added monitoring endpoint #100

Merged

Conversation

p-strusiewiczsurmacki-mobica
Copy link
Contributor

@p-strusiewiczsurmacki-mobica p-strusiewiczsurmacki-mobica commented Feb 28, 2024

This PR adds monitoring endpoints as described in #96.

  • show ip/ipv6 route (vrf <vrf>) <input> json (with/without longer-prefixes)
  • show bgp (vrf <vrf>) ipv4/ipv6 unicast <input> json (with/without longer-prefixes)
  • show evpn vni json
  • show evpn rmac vni <all|vrf> json
  • show evpn mac vni <all|vrf> json
  • show evpn next-hops vni <all|vrf> json
  • show bgp vrf <all|vrf> summary json

Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
@p-strusiewiczsurmacki-mobica p-strusiewiczsurmacki-mobica changed the title Added monitoring endpoint [WIP] Added monitoring endpoint Feb 28, 2024
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
pkg/monitoring/endpoint.go Show resolved Hide resolved
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
@p-strusiewiczsurmacki-mobica p-strusiewiczsurmacki-mobica marked this pull request as ready for review March 13, 2024 11:56
@p-strusiewiczsurmacki-mobica p-strusiewiczsurmacki-mobica changed the title [WIP] Added monitoring endpoint Added monitoring endpoint Mar 13, 2024
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
config/manager/service.yaml Outdated Show resolved Hide resolved
config/manager/service.yaml Outdated Show resolved Hide resolved
config/rbac/role.yaml Show resolved Hide resolved
config/rbac/role.yaml Show resolved Hide resolved
pkg/monitoring/endpoint.go Show resolved Hide resolved
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
@schrej schrej requested a review from chdxD1 April 17, 2024 12:47
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
return
}

data := e.cli.ExecuteWithJSON(command)
Copy link
Member

Choose a reason for hiding this comment

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

Is this safe? We're directly passing query args (vrf) to the command. Do we need to do some additional sanitization here to prevent RCE? I think making sure it's only allowed characters would go a long way.

Same applies to all other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. However I am not sure what values can be used for VRF/VNI, so for now I've just restricted those to:

regexp.MustCompile("^[a-zA-Z0-9_]")

Should I add something to it? Is dash - valid character for this, for example? Or maybe underscore _ should be removed?

I believe, all the other parameters are sanitized in switch/if statements.

Copy link
Member

Choose a reason for hiding this comment

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

dash and underscore should both be valid parameters here. VNI is a 24-bit number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, changed VRF regexp to
"^[a-zA-Z0-9-_.]+$"

And added validation function for VNI:

func validateVNI(vni string) error {

pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
pkg/monitoring/endpoint.go Outdated Show resolved Hide resolved
Separated passRequest

Co-authored-by: Jakob Schrettenbrunner <[email protected]>
…rate docker images and contianers in pod.

Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Copy link
Member

@schrej schrej left a comment

Choose a reason for hiding this comment

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

lgtm, please wait for feedback on that validation comment from @chdxD1

Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
@chdxD1 chdxD1 merged commit 9de1c88 into telekom:main May 24, 2024
3 checks passed
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.

4 participants