Skip to content

Commit

Permalink
Check for invalid certs
Browse files Browse the repository at this point in the history
Summary:
This diff is part 4 of the stacked changes to introduce a new health-report command in Eden. Specifically, this diff focuses on adding checks to notify if certs are invalid.

### Eden Health Check Subcommand
**Problem Statement**
Currently, when Eden runs within VSCode, we are unable to effectively surface Eden-related issues/failures to users who are unaware that Eden is not healthy on their machine. This can lead to users being unable to use Eden-dependent services like Myles and Sapling.

**Solution**
To address this issue, we propose creating a new subcommand that performs critical checks related to Eden health. This subcommand will be lightweight and can be automated via VSCode periodic runs.

**Critical Checks**
The following critical checks should be included in the subcommand:
- Eden Daemon Not Running: Check if the Eden daemon is running using eden run.
- No Repo Mount Found: Check if any repositories are mounted and if the user is actively developing using Eden-checked out repos.
- Corp/Prod Canal Certificates Missing/Expired: Check if Corp/Prod canal certificates are missing or expired, which would prevent Eden from operating.
- Eden Stale Version: Check if the Eden version is older than 28 days from the current deployed or installed version.
- Last Chef Age/Run: Check if Chef has run recently (ideally every 30 minutes) to ensure that the latest Eden version is installed.

**Benefits**
By implementing this subcommand, we can:
- Surface Eden-related issues/failures to users more effectively
- Improve user engagement with Eden-dependent services like Myles and Sapling
- Reduce the likelihood of users blaming other systems for issues caused by Eden
- Ensure that users have the latest version of Eden installed and running on their machines

Reviewed By: lXXXw

Differential Revision: D65494875

fbshipit-source-id: fddb5cc7032ea7fcd347568d342fba35653f9103
  • Loading branch information
Vini Gupta authored and facebook-github-bot committed Nov 13, 2024
1 parent d4a377c commit fbb0f2b
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 12 deletions.
21 changes: 14 additions & 7 deletions eden/fs/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from typing import Dict, List, Optional, Set, Tuple, Type

import thrift.transport
from eden.fs.cli.version import VersionInfo

try:
from cli.py import par_telemetry
Expand All @@ -43,14 +44,14 @@ def set_sample_rate(self, automation: int) -> None:

from eden.fs.cli.buck import get_buck_command, run_buck_command
from eden.fs.cli.config import HG_REPO_TYPES
from eden.fs.cli.doctor.facebook import check_x509
from eden.fs.cli.telemetry import TelemetrySample
from eden.fs.cli.util import (
check_health_using_lockfile,
EdenStartError,
is_apple_silicon,
wait_for_instance_healthy,
)
from eden.fs.cli.version import VersionInfo
from eden.thrift.legacy import EdenClient, EdenNotRunningError
from facebook.eden import EdenService
from facebook.eden.ttypes import ChangeOwnershipRequest, MountState
Expand Down Expand Up @@ -1198,11 +1199,13 @@ class HealthReportCmd(Subcmd):
class ErrorCode(Enum):
EDEN_NOT_RUNNING = 1
STALE_EDEN_VERSION = 2
INVALID_CERTS = 3

def description(self) -> str:
descriptions = {
self.EDEN_NOT_RUNNING: "The EdenFS daemon is not running",
self.STALE_EDEN_VERSION: "The running EdenFS daemon is over 30 days out-of-date",
self.INVALID_CERTS: "The EdenFS cannot find a valid user certificate",
}
return descriptions[self]

Expand All @@ -1229,7 +1232,6 @@ def is_eden_running(self, instance: EdenInstance) -> bool:
return True

def is_eden_up_to_date(self) -> bool:
# if running EdenFS version is stale
if (
self.version_info.ages_deltas is not None
and self.version_info.ages_deltas >= 30
Expand All @@ -1238,6 +1240,13 @@ def is_eden_up_to_date(self) -> bool:
return False
return True

def are_certs_valid(self) -> bool:
if (cert := check_x509.find_x509_path()) and check_x509.validate_x509(cert):
return True
# cert error!
self.error_codes.add(HealthReportCmd.ErrorCode.INVALID_CERTS)
return False

@staticmethod
def print_error_codes_json(out: ui.Output) -> None:
"""
Expand All @@ -1261,12 +1270,10 @@ def run(self, args: argparse.Namespace) -> int:
out = ui.get_output()
exit_code = 0

if not self.is_eden_running(instance):
if not self.is_eden_running(instance) or not all(
f() for f in [self.is_eden_up_to_date, self.are_certs_valid]
):
exit_code = 1
else:
is_up_to_date = self.is_eden_up_to_date()
if not is_up_to_date:
exit_code = 1

self.print_error_codes_json(out)

Expand Down
57 changes: 52 additions & 5 deletions eden/fs/cli/test/health_report_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from datetime import datetime
from unittest.mock import MagicMock, patch

from eden.fs.cli.version import date_from_version, VersionInfo
from eden.fs.cli.version import VersionInfo

from eden.test_support.temporary_directory import TemporaryDirectoryMixin

Expand Down Expand Up @@ -85,6 +85,8 @@ def setup(self) -> typing.Tuple[MagicMock, argparse.Namespace]:
mock_argument_parser = MagicMock(spec=argparse.ArgumentParser)
return (mock_argument_parser, args)

@patch("eden.fs.cli.doctor.facebook.check_x509.find_x509_path")
@patch("eden.fs.cli.doctor.facebook.check_x509.validate_x509")
@patch("eden.fs.cli.config.EdenInstance.get_running_version")
@patch("eden.fs.cli.version.get_version_info")
@patch("eden.fs.cli.util.HealthStatus.is_healthy")
Expand All @@ -93,12 +95,16 @@ def test_calling_into_health_report(
mock_is_healthy: MagicMock,
mock_get_version_info: MagicMock,
mock_get_running_version: MagicMock,
mock_validate_x509: MagicMock,
mock_find_x509_path: MagicMock,
) -> None:
mock_argument_parser, args = self.setup()

mock_get_running_version.return_value = latest_version
mock_get_version_info.return_value = latest_running_version_info
mock_is_healthy.return_value = True
mock_find_x509_path.return_value = ("some_cert_path",)
mock_validate_x509.return_value = True

test_health_report_cmd = HealthReportCmd(mock_argument_parser)
result = test_health_report_cmd.run(args)
Expand All @@ -121,19 +127,25 @@ def test_health_report_notify_eden_not_running(

self.assertEqual(result, 1)

@patch("eden.fs.cli.doctor.facebook.check_x509.find_x509_path")
@patch("eden.fs.cli.doctor.facebook.check_x509.validate_x509")
@patch("eden.fs.cli.config.EdenInstance.get_running_version")
@patch("eden.fs.cli.version.get_version_info")
@patch("eden.fs.cli.util.HealthStatus.is_healthy")
def test_health_report_check_for_stale_eden_version_prompt_error(
self,
mock_get_running_version: MagicMock,
mock_get_version_info: MagicMock,
mock_is_healthy: MagicMock,
mock_get_version_info: MagicMock,
mock_get_running_version: MagicMock,
mock_validate_x509: MagicMock,
mock_find_x509_path: MagicMock,
) -> None:
mock_argument_parser, args = self.setup()
mock_get_running_version.return_value = stale_version
mock_get_version_info.return_value = stale_running_version_info
mock_is_healthy.return_value = True
mock_find_x509_path.return_value = ("some_cert_path",)
mock_validate_x509.return_value = True

test_health_report_cmd = HealthReportCmd(mock_argument_parser)
result = test_health_report_cmd.run(args)
Expand All @@ -142,21 +154,56 @@ def test_health_report_check_for_stale_eden_version_prompt_error(
)
self.assertEqual(result, 1)

@patch("eden.fs.cli.doctor.facebook.check_x509.find_x509_path")
@patch("eden.fs.cli.doctor.facebook.check_x509.validate_x509")
@patch("eden.fs.cli.config.EdenInstance.get_running_version")
@patch("eden.fs.cli.version.get_version_info")
@patch("eden.fs.cli.util.HealthStatus.is_healthy")
def test_health_report_check_for_stale_eden_version_no_error(
self,
mock_get_running_version: MagicMock,
mock_get_version_info: MagicMock,
mock_is_healthy: MagicMock,
mock_get_version_info: MagicMock,
mock_get_running_version: MagicMock,
mock_validate_x509: MagicMock,
mock_find_x509_path: MagicMock,
) -> None:
mock_argument_parser, args = self.setup()
mock_get_running_version.return_value = acceptable_version
mock_get_version_info.return_value = acceptable_running_version_info
mock_is_healthy.return_value = True
mock_find_x509_path.return_value = ("some_cert_path",)
mock_validate_x509.return_value = True

test_health_report_cmd = HealthReportCmd(mock_argument_parser)
# test_health_report_cmd.error_codes.clear()
result = test_health_report_cmd.run(args)
self.assertEqual(HealthReportCmd.error_codes, set())
self.assertEqual(result, 0)

@patch("eden.fs.cli.doctor.facebook.check_x509.find_x509_path")
@patch("eden.fs.cli.doctor.facebook.check_x509.validate_x509")
@patch("eden.fs.cli.config.EdenInstance.get_running_version")
@patch("eden.fs.cli.version.get_version_info")
@patch("eden.fs.cli.util.HealthStatus.is_healthy")
def test_health_report_check_for_invalid_certs(
self,
mock_is_healthy: MagicMock,
mock_get_version_info: MagicMock,
mock_get_running_version: MagicMock,
mock_validate_x509: MagicMock,
mock_find_x509_path: MagicMock,
) -> None:
mock_argument_parser, args = self.setup()
mock_find_x509_path.return_value = ("some_cert_path",)
mock_validate_x509.return_value = False
mock_get_running_version.return_value = acceptable_version
mock_get_version_info.return_value = acceptable_running_version_info
mock_is_healthy.return_value = True

test_health_report_cmd = HealthReportCmd(mock_argument_parser)
result = test_health_report_cmd.run(args)
self.assertEqual(
HealthReportCmd.error_codes, {HealthReportCmd.ErrorCode.INVALID_CERTS}
)

self.assertEqual(result, 1)

0 comments on commit fbb0f2b

Please sign in to comment.