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: adjust thresholds for certificate health #107

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

bhumitra
Copy link
Contributor

@bhumitra bhumitra commented Mar 22, 2023

In order to have a good experience with our community, we recommend that you read the contributing guidelines for making a pull request.

Summary of Pull Request

  • Adjust thresholds for certificate health expiry
  • Added ESXi hosts data back into the report
  • Added certificate expires in days column to report

Type of Pull Request

  • This is a bug fix.
  • This is an enhancement or feature.
  • This is a code style / formatting update.
  • This is a documentation update.
  • This is a refactoring update.
  • This is a chore update
  • This is something else.
    Please describe:

Related to Existing Issues

Closes #101

Test and Documentation Coverage

  • Tests have been completed (for bug fixes / features).
  • Documentation has been added / updated (for bug fixes / features).

Breaking Changes?

  • Yes, there are breaking changes.
  • No, there are no breaking changes.

Signed-off-by: bhumitra nagar <bnagar@vmware.com>
@bhumitra bhumitra requested a review from a team as a code owner March 22, 2023 22:39
@github-actions github-actions bot added the needs-review Needs Review label Mar 22, 2023
@bhumitra
Copy link
Contributor Author

image

This is what it looks like now.

@tenthirtyam tenthirtyam self-requested a review March 23, 2023 03:32
@tenthirtyam tenthirtyam changed the title feat: Adjust thresholds for certificate health expiry feat: adjust thresholds for certificate health expiry Mar 23, 2023
Copy link
Contributor

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@tenthirtyam tenthirtyam added enhancement Enhancement report/health Health Report and removed needs-review Needs Review labels Mar 23, 2023
@tenthirtyam tenthirtyam added this to the v2.0.0 milestone Mar 23, 2023
Copy link
Contributor

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

Error observed on Windows:

PS F:\VMware.CloudFoundation.Reporting> Import-Module .\VMware.CloudFoundation.Reporting.psd1
PS F:\VMware.CloudFoundation.Reporting> Publish-CertificateHealth -json F:\Reporting\HealthReports\sfo-w01-all-health-results.json
 [03-28-2023_14:17:05]   Error at Script Line 926
 [03-28-2023_14:17:05]   Relevant Command: $expiry = [DateTime]::ParseExact($element.title[2], "MMM dd HH:mm:ss yyyy 'GMT'", $null).ToUniversalTime()
 [03-28-2023_14:17:05]   Error Message: Exception calling "ParseExact" with "3" argument(s): "String was not recognized as a valid DateTime."
PS F:\VMware.CloudFoundation.Reporting> Publish-CertificateHealth -json F:\Reporting\HealthReports\sfo-vcf01-all-health-results.json
 [03-28-2023_14:17:22]   Error at Script Line 926
 [03-28-2023_14:17:22]   Relevant Command: $expiry = [DateTime]::ParseExact($element.title[2], "MMM dd HH:mm:ss yyyy 'GMT'", $null).ToUniversalTime()
 [03-28-2023_14:17:22]   Error Message: Exception calling "ParseExact" with "3" argument(s): "String was not recognized as a valid DateTime."

@tenthirtyam tenthirtyam added the size/s Relative Sizing: small label Mar 28, 2023
@tenthirtyam tenthirtyam marked this pull request as draft March 28, 2023 14:40
@tenthirtyam
Copy link
Contributor

Marked as draft whilst awaiting changes.

Signed-off-by: bhumitra nagar <bnagar@vmware.com>
@tenthirtyam tenthirtyam marked this pull request as ready for review March 28, 2023 17:49
@tenthirtyam
Copy link
Contributor

Looks good after testing with the update but I do see an oddity where it’s reporting a -25 but an 8 day expiration. You can test with the JSON I provided over Slack. I suspect this is a data issue from SoS?

PS F:\VMware.CloudFoundation.Reporting> Publish-CertificateHealth -json F:\Reporting\HealthReports\sfo-vcf01-all-health-results.json -failureOnly


Component         : vRLI
Resource          : sfo-vrli01a.sfo.rainpole.io
Expires In (Days) : -25
Alert             : RED
Message           : Certificate validation is failed for vRLI:sfo-vrli01a.sfo.rainpole.io. If Certificate End date is 15 days or less, SoS will show result as failed.Certificate expires in 8
                    day(s).

@tenthirtyam
Copy link
Contributor

I’ll submit an update for the build and changelog before merge.

@tenthirtyam tenthirtyam self-requested a review March 28, 2023 17:55
@tenthirtyam tenthirtyam dismissed their stale review March 28, 2023 17:55

Next review

@vmwclabot
Copy link
Member

@bhumitra, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@vmwclabot vmwclabot added the dco-required DCO Required label Mar 29, 2023
Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
@tenthirtyam tenthirtyam force-pushed the feat-adjust-thresholds-cert-exp branch from c53b7a6 to 349b76f Compare March 29, 2023 00:29
@vmwclabot vmwclabot removed the dco-required DCO Required label Mar 29, 2023
Bumps the module version to v2.0.0.1003.

Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
Updates `CHANGELOG.md` to include enhancements from vmware#107.

Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
@tenthirtyam tenthirtyam changed the title feat: adjust thresholds for certificate health expiry feat: adjust thresholds for certificate health Mar 29, 2023
Copy link
Contributor

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@tenthirtyam tenthirtyam merged commit 77df666 into vmware:main Mar 29, 2023
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement report/health Health Report size/s Relative Sizing: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust thresholds for certificates health expiry date
3 participants