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

feat(healthendpoint): Refactor health endpoint in all services to run on same port as main service #3028

Open
wants to merge 99 commits into
base: main
Choose a base branch
from

Conversation

bonzofenix
Copy link
Contributor

@bonzofenix bonzofenix commented Jun 16, 2024

Overview

MTLS verification is performed server-side for communication in between App Autoscaler components. To prepare components to run as Cloud Foundry (CF) apps on SAP BTP, we need to merge the health and service ports for each service, as there is no TCP routing enabled. The /health endpoints in each CF Autoscaler component are monitored by our Cloud Engineering Team and must remain available without MTLS requirements. When migrating apps from BOSH VMs to CF, apps can only listen on a single port in the container. Therefore, the health endpoint and / health metrics must be available on the same service of the server.

Whats new

• Basic Authentication Implementation: Introduced basic authentication for multiple components, including scaling engine, event generator, and operator.
• OpenAPI Specification Enhancements: Added new API endpoints for scaling history and improved existing API structures.
• Prometheus Integration: Enhanced health monitoring with Prometheus metrics collection for various services.
• Refactoring and Code Cleanup: Streamlined code by removing unused dependencies and consolidating authentication logic.
• Testing Improvements: Expanded test coverage for new authentication mechanisms and API endpoints.
• Configuration Management: Updated configuration files to reflect new authentication parameters and removed deprecated settings.

Why should we merge this PR in one go

  • Component communication using basic auth would be enabled and running before migrating to CF.
  • No duplication of unit test would be needed supporting mtls+multiple server ports vs Basicauth+single server port

Why we would not want to merge this PR

  • Any consumer relaying on mtls would need to relay on basic auth from now own (MTLS would be deprecated)
  • No rollback option in case of unknown issues (relaying on existing acceptance+performance+integration tests)

Out of scope

  • Scheduler basic auth server support
  • Extracting service broker from the api package to run it as an independent service

@bonzofenix bonzofenix marked this pull request as draft June 16, 2024 12:36
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 2 times, most recently from dc4566c to bedbdbc Compare June 17, 2024 14:25
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 4 times, most recently from 2085fa2 to cbaa084 Compare June 17, 2024 17:15
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 2 times, most recently from ffaf165 to bd90ac7 Compare June 18, 2024 11:04
@bonzofenix bonzofenix self-assigned this Jun 18, 2024
@bonzofenix bonzofenix marked this pull request as ready for review June 18, 2024 16:23
@bonzofenix bonzofenix requested review from geigerj0 and removed request for geigerj0 June 18, 2024 16:50
@bonzofenix bonzofenix changed the title Refactor health endpoint in all services to run on same port as main service feat(healthendpoint): Refactor health endpoint in all services to run on same port as main service Jun 19, 2024
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 10 times, most recently from 41a044c to 636b653 Compare June 25, 2024 17:02
@bonzofenix bonzofenix added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Jun 25, 2024
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 3 times, most recently from 2d9aaba to 9417582 Compare July 3, 2024 12:41
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch from f6d8da4 to d586408 Compare July 25, 2024 12:53
bonzofenix and others added 8 commits July 25, 2024 16:49
 - Remove hardcoded paths and replace with dynamic URL construction using `fmt.Sprintf` and `url.Parse`
 - Consolidate constants for API paths into the tests where they are used
 - Eliminate unused variables and imports following the URL refactoring
 - Update functions to construct URLs dynamically before making HTTP requests
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch 2 times, most recently from 8f7631a to c49eca2 Compare July 29, 2024 11:28
@bonzofenix bonzofenix force-pushed the feature/754-healthendpoint-service-refactor branch from c49eca2 to 45d9256 Compare July 29, 2024 11:34
bonzofenix and others added 6 commits July 29, 2024 13:41
 • Update tests to construct serverUrl as a url.URL object instead of a string.
 • Simplify the request setup by removing redundant helper functions and using setupRequest directly.
 • Set basic auth credentials on the request using SetBasicAuth method.
 • Adjust serverUrl.Path for each test case to point to the correct endpoint.
Copy link

sonarcloud bot commented Aug 6, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. breaking-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants