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

Add gateway edition to healthcheck response #6679

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeffy-mathew
Copy link
Contributor

@jeffy-mathew jeffy-mathew commented Oct 29, 2024

User description

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

enhancement


Description

  • Added a new Edition field to the HealthCheckResponse struct to include edition information in the health check response.
  • Implemented GetEdition function in separate files for Community and Enterprise editions to return the respective edition strings.
  • Updated the liveCheckHandler to populate the Edition field using the GetEdition function.

PRDescriptionHeader.CHANGES_WALKTHROUGH

Relevant files
Enhancement
health_check.go
Add Edition field to HealthCheckResponse struct                   

apidef/health_check.go

  • Added Edition field to HealthCheckResponse struct.
+1/-0     
health_check.go
Include Edition information in health check response         

gateway/health_check.go

  • Imported edition package.
  • Set Edition field in HealthCheckResponse using edition.GetEdition().
  • +2/-0     
    edition_ce.go
    Define GetEdition function for Community edition                 

    internal/edition/edition_ce.go

    • Added GetEdition function returning "Community".
    +7/-0     
    edition_ee.go
    Define GetEdition function for Enterprise edition               

    internal/edition/edition_ee.go

    • Added GetEdition function returning "Enterprise".
    +7/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @buger
    Copy link
    Member

    buger commented Oct 29, 2024

    A JIRA Issue ID is missing from your branch name, PR title and PR description! 🦄

    Your branch: exp/add-edition-info-to-healthcheck

    Your PR title: Add gateway edition to healthcheck response

    Your PR description:

    Description

    Related Issue

    Motivation and Context

    How This Has Been Tested

    Screenshots (if appropriate)

    Types of changes

    • Bug fix (non-breaking change which fixes an issue)
    • New feature (non-breaking change which adds functionality)
    • Breaking change (fix or feature that would cause existing functionality to change)
    • Refactoring or add test (improvements in base code or adds test coverage to functionality)

    Checklist

    • I ensured that the documentation is up to date
    • I explained why this PR updates go.mod in detail with reasoning why it's required
    • I would like a code coverage CI quality gate exception and have explained why

    If this is your first time contributing to this repository - welcome!


    Please refer to jira-lint to get started.

    Without the JIRA Issue ID in your branch name you would lose out on automatic updates to JIRA via SCM; some GitHub status checks might fail.

    Valid sample branch names:

    ‣ feature/shiny-new-feature--mojo-10'
    ‣ 'chore/changelogUpdate_mojo-123'
    ‣ 'bugfix/fix-some-strange-bug_GAL-2345'

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Validate the Edition value to ensure it meets expected criteria before use

    Validate the Edition value retrieved from edition.GetEdition() to ensure it is not
    empty or unexpected before assigning it to the HealthCheckResponse.

    gateway/health_check.go [187]

    -Edition:     edition.GetEdition(),
    +editionValue := edition.GetEdition()
    +if editionValue != "Community" && editionValue != "Enterprise" {
    +    logrus.Error("Invalid edition value")
    +    editionValue = "Unknown"
    +}
    +Edition:     editionValue,
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential bug by ensuring that the Edition value is validated before use, which is crucial for preventing unexpected behavior. It significantly enhances the robustness of the code.

    9

    Copy link
    Contributor

    github-actions bot commented Oct 29, 2024

    API Changes

    --- prev.txt	2024-10-29 07:41:57.696286096 +0000
    +++ current.txt	2024-10-29 07:41:51.312279831 +0000
    @@ -1862,6 +1862,7 @@
     	Output      string                     `json:"output,omitempty"`
     	Description string                     `json:"description,omitempty"`
     	Details     map[string]HealthCheckItem `json:"details,omitempty"`
    +	Edition     string                     `json:"edition,omitempty"`
     }
     
     type HealthCheckStatus string

    @jeffy-mathew jeffy-mathew requested a review from a team as a code owner October 29, 2024 07:40
    Copy link

    Quality Gate Failed Quality Gate failed

    Failed conditions
    0.0% Coverage on New Code (required ≥ 80%)

    See analysis details on SonarCloud

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants