From e29ff3bbe4289486b829f13bc47689edadb8a7a6 Mon Sep 17 00:00:00 2001 From: Carlos Monastyrski Date: Tue, 23 Sep 2025 11:40:02 -0300 Subject: [PATCH] Few improvements on version upgrade tool --- .github/workflows/validate-upgrade-path.yml | 212 ++++++++++++++++++ .../server/routes/v1/upgrade-path-router.ts | 7 +- .../services/upgrade-path/github-client.ts | 4 + .../upgrade-path/upgrade-path-service.ts | 71 +++--- 4 files changed, 266 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/validate-upgrade-path.yml diff --git a/.github/workflows/validate-upgrade-path.yml b/.github/workflows/validate-upgrade-path.yml new file mode 100644 index 000000000..4a6812297 --- /dev/null +++ b/.github/workflows/validate-upgrade-path.yml @@ -0,0 +1,212 @@ +name: "Validate Upgrade Path Configuration" + +on: + pull_request: + types: [opened, synchronize] + paths: + - "backend/upgrade-path.yaml" + + workflow_call: + +jobs: + validate-upgrade-path: + name: Validate upgrade-path.yaml format + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout source + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for changes in upgrade-path.yaml + id: check-changes + run: | + # For local testing with act, always run validation + if [ "${ACT:-false}" = "true" ]; then + echo "changed=true" >> $GITHUB_OUTPUT + echo "Running validation (local act mode)" + else + # Check if upgrade-path.yaml was modified in this PR + if git diff --name-only HEAD^ HEAD | grep -q "backend/upgrade-path.yaml"; then + echo "changed=true" >> $GITHUB_OUTPUT + echo "Changes detected in backend/upgrade-path.yaml" + else + echo "changed=false" >> $GITHUB_OUTPUT + echo "No changes detected in backend/upgrade-path.yaml" + fi + fi + + - name: Setup Python for YAML validation + if: steps.check-changes.outputs.changed == 'true' + run: | + # Use system python3 and install PyYAML + python3 --version || echo "Python3 not found" + python3 -m pip install --user PyYAML || pip3 install PyYAML || echo "PyYAML installation failed" + + - name: Validate upgrade-path.yaml format + if: steps.check-changes.outputs.changed == 'true' + run: | + echo "Running upgrade-path.yaml validation..." + + # Debug: Check if file exists + ls -la backend/upgrade-path.yaml || echo "File not found" + + # Debug: Show last few lines + echo "Last 5 lines of file:" + tail -5 backend/upgrade-path.yaml || echo "Cannot read file" + + echo "Starting Python validation..." + + python3 << 'EOF' + import yaml + import re + import sys + + def validate_upgrade_path(): + try: + print("Reading upgrade-path.yaml...") + with open('backend/upgrade-path.yaml', 'r') as file: + content = file.read() + + if not content.strip(): + raise ValueError("File is empty") + + if len(content) > 1024 * 1024: + raise ValueError("File is too large (>1MB)") + + print("Parsing YAML content...") + try: + config = yaml.safe_load(content) + print("YAML parsed successfully") + except yaml.YAMLError as e: + print(f"YAML parsing failed: {e}") + raise ValueError(f"Invalid YAML syntax: {e}") + + if not isinstance(config, dict): + raise ValueError("Root level must be an object") + + print("YAML syntax is valid") + print("Validating schema structure...") + + if 'versions' not in config: + print("Warning: No versions found in the configuration") + return True + + versions = config['versions'] + if not isinstance(versions, dict): + raise ValueError("'versions' must be an object") + + print(f"Found {len(versions)} version(s) to validate") + + # Version key pattern validation + version_pattern = re.compile(r'^[a-zA-Z0-9._/-]+$') + common_patterns = [ + re.compile(r'^v?\d+\.\d+\.\d+$'), # v1.2.3 or 1.2.3 + re.compile(r'^v?\d+\.\d+\.\d+\.\d+$'), # v1.2.3.4 or 1.2.3.4 + re.compile(r'^infisical/v?\d+\.\d+\.\d+$'), # infisical/v1.2.3 + re.compile(r'^infisical/v?\d+\.\d+\.\d+-\w+$') # infisical/v1.2.3-postgres + ] + + errors = [] + + for version_key, version_config in versions.items(): + print(f"Validating version key: {version_key}") + + # Validate version key format + if not version_pattern.match(version_key): + errors.append(f"Invalid version key '{version_key}': contains invalid characters") + continue + + if len(version_key) > 50: + errors.append(f"Version key '{version_key}' is too long (max 50 characters)") + continue + + if not isinstance(version_config, dict): + errors.append(f"Version '{version_key}' configuration must be an object") + continue + + # Validate breaking_changes + if 'breaking_changes' in version_config: + breaking_changes = version_config['breaking_changes'] + if not isinstance(breaking_changes, list): + errors.append(f"Version '{version_key}': breaking_changes must be a list") + elif len(breaking_changes) == 0: + errors.append(f"Version '{version_key}': breaking_changes is empty (remove field or add items)") + else: + for i, change in enumerate(breaking_changes): + if not isinstance(change, dict): + errors.append(f"Version '{version_key}': breaking_changes[{i}] must be an object") + continue + + # Validate required fields + for field in ['title', 'description', 'action']: + if field not in change: + errors.append(f"Version '{version_key}': breaking_changes[{i}] missing '{field}'") + elif not isinstance(change[field], str): + errors.append(f"Version '{version_key}': breaking_changes[{i}].{field} must be string") + elif not change[field].strip(): + errors.append(f"Version '{version_key}': breaking_changes[{i}].{field} cannot be empty") + elif field == 'title' and len(change[field]) > 200: + errors.append(f"Version '{version_key}': breaking_changes[{i}].title too long (max 200)") + elif field == 'description' and len(change[field]) > 1000: + errors.append(f"Version '{version_key}': breaking_changes[{i}].description too long (max 1000)") + elif field == 'action' and len(change[field]) > 500: + errors.append(f"Version '{version_key}': breaking_changes[{i}].action too long (max 500)") + + # Validate db_schema_changes + if 'db_schema_changes' in version_config: + db_changes = version_config['db_schema_changes'] + if not isinstance(db_changes, str): + errors.append(f"Version '{version_key}': db_schema_changes must be string") + elif db_changes == "": + errors.append(f"Version '{version_key}': db_schema_changes is empty (remove field or add content)") + elif len(db_changes) > 1000: + errors.append(f"Version '{version_key}': db_schema_changes too long (max 1000)") + + # Validate notes + if 'notes' in version_config: + notes = version_config['notes'] + if not isinstance(notes, str): + errors.append(f"Version '{version_key}': notes must be string") + elif notes == "": + errors.append(f"Version '{version_key}': notes is empty (remove field or add content)") + elif len(notes) > 2000: + errors.append(f"Version '{version_key}': notes too long (max 2000)") + + # Check if version follows common patterns + is_common_pattern = any(pattern.match(version_key) for pattern in common_patterns) + if not is_common_pattern: + print(f"Warning: Version key '{version_key}' doesn't match common patterns. This may be intentional.") + + print(f"Version '{version_key}' is valid") + + if errors: + print("Validation failed with the following errors:") + for error in errors: + print(f" - {error}") + return False + + print("All validations passed!") + print("upgrade-path.yaml format is valid") + return True + + except Exception as e: + print(f"Validation failed: {e}") + return False + + if not validate_upgrade_path(): + sys.exit(1) + EOF + + - name: Validation completed + if: steps.check-changes.outputs.changed == 'true' + run: | + echo "upgrade-path.yaml validation passed!" + echo "The configuration file follows the expected format and all version entries are valid." + + - name: Skipping validation + if: steps.check-changes.outputs.changed == 'false' + run: | + echo "Skipping validation - no changes detected in backend/upgrade-path.yaml" \ No newline at end of file diff --git a/backend/src/server/routes/v1/upgrade-path-router.ts b/backend/src/server/routes/v1/upgrade-path-router.ts index e431e60d5..484b12f53 100644 --- a/backend/src/server/routes/v1/upgrade-path-router.ts +++ b/backend/src/server/routes/v1/upgrade-path-router.ts @@ -2,6 +2,7 @@ import RE2 from "re2"; import { z } from "zod"; import { BadRequestError } from "@app/lib/errors"; +import { logger } from "@app/lib/logger"; import { publicEndpointLimit } from "@app/server/config/rateLimiter"; const versionSchema = z @@ -40,7 +41,7 @@ export const registerUpgradePathRouter = async (server: FastifyZodProvider) => { versions }; } catch (error) { - req.log.error(error, "Failed to fetch versions"); + logger.error(error, "Failed to fetch versions"); if (error instanceof z.ZodError) { throw new BadRequestError({ message: "Invalid query parameters" }); } @@ -101,14 +102,14 @@ export const registerUpgradePathRouter = async (server: FastifyZodProvider) => { const result = await req.server.services.upgradePath.calculateUpgradePath(fromVersion, toVersion); - req.log.info( + logger.info( { pathLength: result.path.length, hasBreaking: result.breakingChanges.length > 0 }, "Upgrade path calculated" ); return result; } catch (error) { - req.log.error(error, "Failed to calculate upgrade path"); + logger.error(error, "Failed to calculate upgrade path"); if (error instanceof z.ZodError) { throw new BadRequestError({ message: `Invalid input: ${error.errors.map((e) => e.message).join(", ")}` }); } diff --git a/backend/src/services/upgrade-path/github-client.ts b/backend/src/services/upgrade-path/github-client.ts index 2982e297a..3fcd5593f 100644 --- a/backend/src/services/upgrade-path/github-client.ts +++ b/backend/src/services/upgrade-path/github-client.ts @@ -121,6 +121,10 @@ const makeRequest = async ( clearTimeout(timeout); if (error instanceof Error && error.name === "AbortError") { + if (retryCount < config.maxRetries) { + await delay(config.retryDelay * 2 ** retryCount); + return await makeRequest(url, config, retryCount + 1); + } throw new Error(`Request timeout after ${config.timeout}ms`); } diff --git a/backend/src/services/upgrade-path/upgrade-path-service.ts b/backend/src/services/upgrade-path/upgrade-path-service.ts index f966e670e..b12eb52d3 100644 --- a/backend/src/services/upgrade-path/upgrade-path-service.ts +++ b/backend/src/services/upgrade-path/upgrade-path-service.ts @@ -5,6 +5,7 @@ import RE2 from "re2"; import { z } from "zod"; import { TKeyStoreFactory } from "@app/keystore/keystore"; +import { logger } from "@app/lib/logger"; import { fetchReleases } from "./github-client"; import { BreakingChange, FormattedRelease, UpgradePathConfig, UpgradePathResult, VersionConfig } from "./types"; @@ -20,25 +21,44 @@ const versionSchema = z .max(50) .regex(new RE2(/^[a-zA-Z0-9._/-]+$/), "Invalid version format"); +const breakingChangeSchema = z.object({ + title: z.string().min(1).max(200), + description: z.string().min(1).max(1000), + action: z.string().min(1).max(500) +}); + +const versionConfigSchema = z.object({ + breaking_changes: z.array(breakingChangeSchema).optional(), + db_schema_changes: z.string().max(1000).optional(), + notes: z.string().max(2000).optional() +}); + interface CalculateUpgradePathParams { fromVersion: string; toVersion: string; } export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFactory) => { + const sanitizeCacheKey = (key: string): string => { + return key.replace(new RE2(/[^a-zA-Z0-9\-:._]/g), "_"); + }; const getGitHubReleases = async (): Promise => { const cacheKey = "upgrade-path:releases"; try { const cached = await keyStore.getItem(cacheKey); - if (cached) return JSON.parse(cached) as FormattedRelease[]; + if (cached) { + const cachedReleases = JSON.parse(cached) as FormattedRelease[]; + if (cachedReleases.length > 0) { + return cachedReleases; + } + } } catch (error) { - // Cache miss, continue to fetch from source + logger.error(error, "Failed to retrieve releases from cache"); } try { const releases = await fetchReleases(false); - const filteredReleases = releases.filter((v) => !v.tagName.includes("nightly")); await keyStore.setItemWithExpiry(cacheKey, 24 * 60 * 60, JSON.stringify(filteredReleases)); @@ -48,14 +68,14 @@ export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFacto } }; - const getUpgradePathConfig = async (): Promise> => { + const getUpgradePathConfig = async (): Promise>> => { const cacheKey = "upgrade-path:config"; try { const cached = await keyStore.getItem(cacheKey); if (cached) return JSON.parse(cached) as Record; } catch (error) { - // Cache miss, continue to fetch from source + logger.error(error, "Failed to retrieve config from cache"); } try { @@ -88,14 +108,12 @@ export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFacto }; const normalizeVersion = (version: string): string => { - // Extract just the X.X.X.X part from any version format const versionRegex = new RE2(/(\d+\.\d+\.\d+(?:\.\d+)?)/); const versionMatch = version.match(versionRegex); if (versionMatch) { return versionMatch[1]; } - // Handle legacy version formats if (version.startsWith("infisical/")) { return version.replace(new RE2(/^infisical\/v?/), "").replace(new RE2(/-[a-zA-Z]+$/), ""); } @@ -104,23 +122,28 @@ export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFacto const findBreakingChangesForVersion = ( version: FormattedRelease, - config: Record + config: Record> ): BreakingChange[] => { // Check multiple key variations for breaking changes configuration const versionNumber = normalizeVersion(version.tagName); + const cleanVersionNumber = versionNumber.replace(new RE2(/^v/), ""); + const possibleKeys = [ version.tagName, version.normalizedTagName, versionNumber, - `v${versionNumber}`, + `v${cleanVersionNumber}`, + cleanVersionNumber, version.tagName.replace(new RE2(/^infisical\//), ""), - version.tagName.replace(new RE2(/^infisical\/v?/), "").replace(new RE2(/-[a-zA-Z]+$/), "") + version.tagName.replace(new RE2(/^infisical\/v?/), "").replace(new RE2(/-[a-zA-Z]+$/), ""), + `v${cleanVersionNumber}`, + cleanVersionNumber ]; for (const key of possibleKeys) { const versionConfig = config[key]; if (versionConfig?.breaking_changes?.length) { - return versionConfig.breaking_changes; + return versionConfig.breaking_changes as BreakingChange[]; } } return []; @@ -145,13 +168,13 @@ export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFacto const calculateUpgradePath = async (params: CalculateUpgradePathParams): Promise => { const { fromVersion, toVersion } = validateParams(params); - const cacheKey = `upgrade-path:${fromVersion}:${toVersion}`; + const cacheKey = sanitizeCacheKey(`upgrade-path:${fromVersion}:${toVersion}`); try { const cached = await keyStore.getItem(cacheKey); if (cached) return JSON.parse(cached) as UpgradePathResult; } catch (error) { - // Cache miss, continue to fetch from source + logger.error(error, "Failed to retrieve upgrade path from cache"); } const [releases, config] = await Promise.all([getGitHubReleases(), getUpgradePathConfig()]); @@ -197,23 +220,21 @@ export const upgradePathServiceFactory = ({ keyStore }: TUpgradePathServiceFacto const features: Array<{ version: string; name: string; body: string; publishedAt: string }> = []; let hasDbMigration = false; - // Process versions in upgrade path, excluding starting version - for (let i = 1; i < upgradePath.length; i += 1) { + // Process versions in upgrade path + for (let i = 0; i < upgradePath.length; i += 1) { const version = upgradePath[i]; const isFromVersion = normalizeVersion(version.normalizedTagName) === cleanFrom; - // Process breaking changes for intermediate versions only - if (!isFromVersion) { - const versionBreakingChanges = findBreakingChangesForVersion(version, config); - if (versionBreakingChanges.length > 0) { - breakingChanges.push({ - version: version.tagName, - changes: versionBreakingChanges - }); - } + // Process breaking changes for all versions in the upgrade path + const versionBreakingChanges = findBreakingChangesForVersion(version, config); + if (versionBreakingChanges.length > 0) { + breakingChanges.push({ + version: version.tagName, + changes: versionBreakingChanges + }); } - // Process database migrations for intermediate versions only + // Process database migrations for intermediate versions only (excluding starting version) if (!isFromVersion) { const versionNumber = normalizeVersion(version.tagName); const possibleKeys = [