-
Notifications
You must be signed in to change notification settings - Fork 8
add new resource schemas #511
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces several new resource schemas for a validation library, each corresponding to different infrastructure resource types such as Kubernetes clusters, namespaces, machines, networks, storage buckets, databases, and file systems. Each resource kind is defined using the Zod schema validation library, leveraging a new generic Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ResourceSchema
participant createKind
User->>ResourceSchema: Import resource kind (e.g., Machine, Bucket)
ResourceSchema->>createKind: Call with version, kind, config, metadata schemas
createKind-->>ResourceSchema: Return Zod schema for resource kind
User-->>ResourceSchema: Validate resource object
ResourceSchema-->>User: Validation result (success/failure)
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/validators/src/resources/kinds/create-kind.ts (1)
6-7
: Consider using more specific type parameters instead ofany
While using
any
for Zod object type parameters works fine, it reduces type safety. Consider using more specific type parameters if possible:- config: z.ZodObject<any, any, any, any, any>; - metadata: z.ZodObject<any, any, any, any, any>; + config: z.ZodObject<z.ZodRawShape, z.ZodTypeAny, z.ZodTypeAny>; + metadata: z.ZodObject<z.ZodRawShape, z.ZodTypeAny, z.ZodTypeAny>;This provides better type checking while maintaining the same functionality.
packages/validators/src/resources/kinds/storage-v1/file-system.ts (1)
14-16
: Consider adding more comprehensive metadata fieldsThe metadata only includes a version field. Consider adding more metadata fields for better resource management:
metadata: z.object({ "file-system/version": z.string(), + "file-system/status": z.string(), + "file-system/region": z.string(), + "file-system/created-at": z.string().datetime().optional(), }),packages/validators/src/resources/kinds/network-v1/network.ts (1)
14-30
: Consider adding default regions or region validationThe schema allows any string for region fields across all provider types. Consider adding validation for regions or providing defaults based on the provider:
z.object({ type: z.literal("aws"), accountId: z.string(), - region: z.string(), + region: z.string().refine(val => AWS_REGIONS.includes(val), { + message: "Must be a valid AWS region" + }), }),Similar validations can be added for Azure and Google Cloud regions.
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts (1)
57-77
: Consistent kind creation with appropriate metadata fields.The use of
createKind
function ensures consistent schema structure across resource types. The metadata schema includes all relevant Kubernetes-specific fields and correctly uses.partial()
to make them optional.Note that
"kubernetes/autoscaling-enabled"
is not marked as optional here (unlike in the namespace schema). Consider whether this should be optional to maintain consistency with other metadata implementations.- "kubernetes/autoscaling-enabled": z.string(), + "kubernetes/autoscaling-enabled": z.string().optional(),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/validators/src/resources/index.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/machine.ts
(1 hunks)packages/validators/src/resources/kinds/create-kind.ts
(1 hunks)packages/validators/src/resources/kinds/network-v1/network.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/bucket.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/database.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/file-system.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}
: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/validators/src/resources/index.ts
packages/validators/src/resources/kinds/storage-v1/file-system.ts
packages/validators/src/resources/kinds/network-v1/network.ts
packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts
packages/validators/src/resources/kinds/create-kind.ts
packages/validators/src/resources/kinds/compute-v1/machine.ts
packages/validators/src/resources/kinds/storage-v1/bucket.ts
packages/validators/src/resources/kinds/storage-v1/database.ts
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
🧬 Code Graph Analysis (5)
packages/validators/src/resources/kinds/storage-v1/file-system.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-15)
packages/validators/src/resources/kinds/network-v1/network.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-15)
packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts (1)
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts (1)
clusterConfig
(5-55)
packages/validators/src/resources/kinds/storage-v1/database.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-15)
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-15)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (7)
packages/validators/src/resources/kinds/storage-v1/database.ts (1)
10-13
: Engine and version fields are duplicated in config and metadataThe
engine
andversion
fields appear in both the config object and metadata object. This could lead to inconsistencies if they're set to different values. Consider:
- Keeping only one source of truth
- Adding validation to ensure they match
- Documenting the purpose of each if they serve different functions
metadata: z.object({ - "database/engine": z.string(), - "database/version": z.string(), + "database/engine": z.string().refine( + (val, ctx) => val === ctx.parent.config.engine || "Engines must match"), + "database/version": z.string().refine( + (val, ctx) => val === ctx.parent.config.version || "Versions must match"), "database/status": z.string(), }),packages/validators/src/resources/kinds/storage-v1/bucket.ts (1)
10-11
: Potential for region inconsistencyThe schema has a top-level
region
field and also includes region fields in each provider configuration. This could lead to inconsistencies if different values are specified:config: z.object({ name: z.string(), - region: z.string(), + region: z.string().refine( + (val, ctx) => { + if (!ctx.parent.provider) return true; + const providerRegion = ctx.parent.provider.region; + return !providerRegion || val === providerRegion || "Regions must match"; + }),Consider either eliminating the duplication or adding validation to ensure the regions match.
packages/validators/src/resources/index.ts (1)
7-12
: Well-structured version constants for API resources.The addition of a centralized
versions
constant as a readonly tuple provides a clear reference point for supported API versions. This promotes consistency across resource definitions and simplifies version management.packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts (1)
1-31
: Well-defined Kubernetes namespace schema with proper validation rules.The schema properly extends the cluster configuration with a namespace field and includes comprehensive metadata validation for Kubernetes-specific attributes. The use of literal values for version and kind ensures type safety.
One observation: The schema implementation correctly uses
.partial()
on the metadata fields to make them optional, which aligns with Kubernetes resource patterns where metadata fields are often added incrementally.packages/validators/src/resources/kinds/compute-v1/machine.ts (2)
3-59
: Comprehensive connection method configurations for multiple providers.The connection method schemas are well-structured with appropriate discriminated unions for different cloud providers and SSH authentication methods. The default SSH port (22) is a nice touch for usability.
61-94
: Well-defined machine schema with detailed metadata validation.The machine schema effectively captures various machine properties with appropriate validation:
- Core fields (version, kind, identifier, name) are properly validated
- The
.passthrough()
on the config allows for provider-specific extensions- Metadata includes detailed machine attributes with appropriate enumerations
The classification of machine types into standard, compute, memory, storage, and accelerated categories is particularly useful for resource categorization.
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts (1)
5-55
: Comprehensive cluster configuration schema with multiple authentication methods.The
clusterConfig
schema effectively covers various connection methods for different cloud providers and local configurations. Each connection method has appropriate required fields and optional parameters.The nullish certificate authority data is a good choice since some clusters might not require this for authentication.
name: z.string(), | ||
region: z.string(), | ||
accessKey: z.string(), | ||
secretKey: z.string(), | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider enhancing security for secret key and adding validation for region
The secretKey
field contains sensitive information and is stored as a plain string. Consider:
- Using a reference to a secret stored in a secure vault instead of the actual secret
- Implementing encryption for this field
- Adding validation to ensure it meets security requirements (minimum length, complexity)
Additionally, the region
field could benefit from validation to ensure it's a valid region format or from a list of allowed regions.
name: z.string(),
- region: z.string(),
+ region: z.string().refine(val => VALID_REGIONS.includes(val), {
+ message: "Must be a valid region"
+ }),
accessKey: z.string(),
- secretKey: z.string(),
+ secretKey: z.string().min(8), // Minimum security requirement
🏁 Script executed:
#!/bin/bash
set -eu
echo "Listing files in storage-v1 directory:"
find packages/validators/src/resources/kinds/storage-v1 -maxdepth 1 -type f -printf '%f\n'
echo
echo "Occurrences of 'region' in storage-v1 schemas:"
rg -n "region" packages/validators/src/resources/kinds/storage-v1
echo
echo "Occurrences of 'secretKey' across the validators directory:"
rg -n "secretKey" packages/validators/src
Length of output: 1633
🏁 Script executed:
#!/bin/bash
set -eu
echo "Searching for VALID_REGIONS in repository:"
rg -n "VALID_REGIONS" --color=never
Length of output: 140
Enhance secretKey security and enforce region validation across storage-v1 schemas
We’ve verified that all three storage-v1 resources—file-system.ts, bucket.ts and database.ts—currently accept region: z.string()
and secretKey: z.string()
with no guarantees on format, length or source. To mitigate risk and prevent invalid regions, please:
• Introduce a shared set of allowed regions (e.g. a VALID_REGIONS
array or a z.enum([...])
)
• Apply region validation against that list in each schema
• Swap plain secretKey
for either a secure vault reference type or enforce encryption at rest
• Add minimum length & complexity checks on the secretKey
field
Example diff for one schema:
-import { z } from "zod"
+import { z } from "zod"
+import { VALID_REGIONS } from "../../../constants/regions"
export const FileSystemConfig = z.object({
- region: z.string(),
+ region: z.string().refine(val => VALID_REGIONS.includes(val), {
+ message: "Must be one of: " + VALID_REGIONS.join(", ")
+ }),
accessKey: z.string(),
- secretKey: z.string(),
+ // Either store as SecureReference or enforce encryption & complexity
+ secretKey: z.string()
+ .min(32, { message: "Secret key must be at least 32 characters" })
+ .regex(/[A-Z]/, { message: "Must include an uppercase letter" })
+ .regex(/[a-z]/, { message: "Must include a lowercase letter" })
+ .regex(/[0-9]/, { message: "Must include a number" })
})
Locations needing updates:
- packages/validators/src/resources/kinds/storage-v1/file-system.ts (lines 9–13)
- packages/validators/src/resources/kinds/storage-v1/bucket.ts (lines 19 & 39)
- packages/validators/src/resources/kinds/storage-v1/database.ts (lines 18 & 28)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
name: z.string(), | |
region: z.string(), | |
accessKey: z.string(), | |
secretKey: z.string(), | |
}), | |
import { z } from "zod"; | |
import { VALID_REGIONS } from "../../../constants/regions"; | |
export const FileSystemConfig = z.object({ | |
name: z.string(), | |
region: z.string().refine(val => VALID_REGIONS.includes(val), { | |
message: "Must be one of: " + VALID_REGIONS.join(", ") | |
}), | |
accessKey: z.string(), | |
// Either store as SecureReference or enforce encryption & complexity | |
secretKey: z.string() | |
.min(32, { message: "Secret key must be at least 32 characters" }) | |
.regex(/[A-Z]/, { message: "Must include an uppercase letter" }) | |
.regex(/[a-z]/, { message: "Must include a lowercase letter" }) | |
.regex(/[0-9]/, { message: "Must include a number" }), | |
}); |
|
||
cidr: z.string(), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation for CIDR format
The cidr
field should contain a valid CIDR notation (e.g., "10.0.0.0/16"). Consider adding a validation pattern:
- cidr: z.string(),
+ cidr: z.string().regex(/^([0-9]{1,3}\.){3}[0-9]{1,3}\/([0-9]|[1-2][0-9]|3[0-2])$/,
+ { message: "Must be a valid CIDR notation (e.g., 10.0.0.0/16)" }),
Also, there's an empty line between name
and cidr
fields which could be removed for consistency.
authMethod: z.discriminatedUnion("type", [ | ||
z.object({ type: z.literal("token"), token: z.string() }), | ||
z.object({ | ||
type: z.literal("aws"), | ||
region: z.string(), | ||
clusterIdentifier: z.string(), | ||
accountId: z.string(), | ||
username: z.string().optional(), | ||
password: z.string().optional(), | ||
}), | ||
z.object({ | ||
type: z.literal("google"), | ||
project: z.string(), | ||
instanceName: z.string(), | ||
region: z.string(), | ||
username: z.string().optional(), | ||
password: z.string().optional(), | ||
}), | ||
z.object({ | ||
type: z.literal("azure"), | ||
resourceGroup: z.string(), | ||
serverName: z.string(), | ||
subscriptionId: z.string(), | ||
username: z.string().optional(), | ||
password: z.string().optional(), | ||
}), | ||
z.object({ | ||
type: z.literal("direct"), | ||
host: z.string(), | ||
port: z.number(), | ||
username: z.string(), | ||
password: z.string(), | ||
ssl: z.boolean().optional(), | ||
}), | ||
]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Enhance security for password fields
The schema includes password fields in multiple auth methods without additional security measures. Consider:
- Using references to secrets stored in a secure vault
- Adding validation for password strength
- Documenting how these credentials should be handled securely
For example:
z.object({
type: z.literal("direct"),
host: z.string(),
port: z.number(),
username: z.string(),
- password: z.string(),
+ password: z.string().min(8).refine(
+ val => /[A-Z]/.test(val) && /[a-z]/.test(val) && /[0-9]/.test(val),
+ { message: "Password must contain uppercase, lowercase, and numbers" }
+ ),
ssl: z.boolean().optional(),
}),
This would enforce basic password strength requirements while still allowing for flexibility.
Enforce strong passwords and support secret references in storage auth schemas
To improve security, let’s enhance all password
fields in authMethod
:
- Define a reusable
passwordSchema
, e.g.:const passwordSchema = z.union([ z.string() .min(8) .refine(val => /[A-Z]/.test(val) && /[a-z]/.test(val) && /[0-9]/.test(val), { message: "Must be ≥8 chars with upper, lower & number", }), z.object({ secretRef: z.string() }), // allow vault references ]);
- Apply it across all methods (mandatory or
.optional()
):z.object({ type: z.literal("direct"), host: z.string(), port: z.number(), username: z.string(),
- password: z.string(),
- password: passwordSchema,
ssl: z.boolean().optional(),
}),
// similarly update aws, google, azure: password: passwordSchema.optional()
- Add a note in your docs/README on secure storage and handling of credentials (e.g., environment vars, vault).
This both enforces strength rules and accommodates secret‐management integrations.
<!-- This is an auto-generated comment by CodeRabbit -->
metadata: z.object({ | ||
"bucket/version": z.string(), | ||
"bucket/status": z.string(), | ||
"bucket/type": z.string(), | ||
"bucket/region": z.string(), | ||
"bucket/access-key": z.string(), | ||
"bucket/secret-key": z.string(), | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storing sensitive credentials in metadata may be a security risk
The metadata schema includes bucket/access-key
and bucket/secret-key
fields. Storing sensitive credentials in metadata is generally not recommended as metadata is often more accessible and may be logged:
metadata: z.object({
"bucket/version": z.string(),
"bucket/status": z.string(),
"bucket/type": z.string(),
"bucket/region": z.string(),
- "bucket/access-key": z.string(),
- "bucket/secret-key": z.string(),
+ "bucket/credential-reference": z.string().optional(), // Reference to externally stored credentials
}),
Consider storing references to credentials rather than the actual values, or using a secure credential storage system.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
metadata: z.object({ | |
"bucket/version": z.string(), | |
"bucket/status": z.string(), | |
"bucket/type": z.string(), | |
"bucket/region": z.string(), | |
"bucket/access-key": z.string(), | |
"bucket/secret-key": z.string(), | |
}), | |
metadata: z.object({ | |
"bucket/version": z.string(), | |
"bucket/status": z.string(), | |
"bucket/type": z.string(), | |
"bucket/region": z.string(), | |
"bucket/credential-reference": z.string().optional(), // Reference to externally stored credentials | |
}), |
provider: z.discriminatedUnion("type", [ | ||
z.object({ | ||
type: z.literal("aws"), | ||
accountId: z.string(), | ||
region: z.string(), | ||
accessKey: z.string(), | ||
secretKey: z.string(), | ||
endpoint: z.string().url().optional(), | ||
}), | ||
z.object({ | ||
type: z.literal("google"), | ||
project: z.string(), | ||
region: z.string(), | ||
serviceAccountKey: z.string(), | ||
}), | ||
z.object({ | ||
type: z.literal("azure"), | ||
storageAccount: z.string(), | ||
resourceGroup: z.string(), | ||
subscriptionId: z.string(), | ||
accessKey: z.string(), | ||
}), | ||
z.object({ | ||
type: z.literal("s3compatible"), | ||
endpoint: z.string().url(), | ||
accessKey: z.string(), | ||
secretKey: z.string(), | ||
region: z.string().optional(), | ||
}), | ||
]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Enhance security for credential fields
The schema includes several sensitive credentials (accessKey, secretKey, serviceAccountKey, etc.) stored as plain strings. Consider:
- Using references to secrets stored in a secure vault
- Adding validation for credential formats where applicable
- Documenting how these credentials should be handled securely
For example:
z.object({
type: z.literal("aws"),
accountId: z.string(),
region: z.string(),
- accessKey: z.string(),
- secretKey: z.string(),
+ accessKey: z.string().regex(/^[A-Z0-9]{20}$/, {
+ message: "AWS Access Key must be 20 uppercase alphanumeric characters"
+ }),
+ secretKey: z.string().min(40), // AWS secret keys are at least 40 characters
endpoint: z.string().url().optional(),
}),
This adds some basic format validation while maintaining flexibility.
Secure raw credential fields in storage schema
File: packages/validators/src/resources/kinds/storage-v1/bucket.ts (lines 13–42)
The Zod schema currently accepts raw credential strings (accessKey, secretKey, serviceAccountKey, etc.) without any guardrails. To reduce risk of accidental leakage or misuse, please:
• Reference secrets stored in a secure vault or via environment‐based secret IDs instead of raw literals
• Add stricter format validations where possible (e.g., AWS Access Keys are 20 uppercase alphanumeric characters, AWS Secret Keys ≥ 40 chars)
• Document how each credential field is provisioned, rotated, and protected in your operational guide
For example:
z.object({
type: z.literal("aws"),
accountId: z.string(),
region: z.string(),
- accessKey: z.string(),
- secretKey: z.string(),
+ // Use a vault reference or validate actual key format
+ accessKey: z.string()
+ .regex(/^[A-Z0-9]{20}$/, {
+ message: "AWS Access Key must be 20 uppercase alphanumeric characters",
+ })
+ .describe("Vault secret ID or direct key retrieved securely"),
+ secretKey: z.string()
+ .min(40, { message: "AWS Secret Key must be at least 40 characters" })
+ .describe("Vault secret ID or direct key retrieved securely"),
endpoint: z.string().url().optional(),
}),
Apply similar vault-reference patterns and validations to your Google, Azure, and S3-compatible provider objects, and include guidance in your README or internal docs on handling these fields securely.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
provider: z.discriminatedUnion("type", [ | |
z.object({ | |
type: z.literal("aws"), | |
accountId: z.string(), | |
region: z.string(), | |
accessKey: z.string(), | |
secretKey: z.string(), | |
endpoint: z.string().url().optional(), | |
}), | |
z.object({ | |
type: z.literal("google"), | |
project: z.string(), | |
region: z.string(), | |
serviceAccountKey: z.string(), | |
}), | |
z.object({ | |
type: z.literal("azure"), | |
storageAccount: z.string(), | |
resourceGroup: z.string(), | |
subscriptionId: z.string(), | |
accessKey: z.string(), | |
}), | |
z.object({ | |
type: z.literal("s3compatible"), | |
endpoint: z.string().url(), | |
accessKey: z.string(), | |
secretKey: z.string(), | |
region: z.string().optional(), | |
}), | |
]), | |
provider: z.discriminatedUnion("type", [ | |
z.object({ | |
type: z.literal("aws"), | |
accountId: z.string(), | |
region: z.string(), | |
// Use a vault reference or validate actual key format | |
accessKey: z.string() | |
.regex(/^[A-Z0-9]{20}$/, { | |
message: "AWS Access Key must be 20 uppercase alphanumeric characters", | |
}) | |
.describe("Vault secret ID or direct key retrieved securely"), | |
secretKey: z.string() | |
.min(40, { message: "AWS Secret Key must be at least 40 characters" }) | |
.describe("Vault secret ID or direct key retrieved securely"), | |
endpoint: z.string().url().optional(), | |
}), | |
z.object({ | |
type: z.literal("google"), | |
project: z.string(), | |
region: z.string(), | |
serviceAccountKey: z.string(), | |
}), | |
z.object({ | |
type: z.literal("azure"), | |
storageAccount: z.string(), | |
resourceGroup: z.string(), | |
subscriptionId: z.string(), | |
accessKey: z.string(), | |
}), | |
z.object({ | |
type: z.literal("s3compatible"), | |
endpoint: z.string().url(), | |
accessKey: z.string(), | |
secretKey: z.string(), | |
region: z.string().optional(), | |
}), | |
]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/validators/src/resources/kinds/storage-v1/database.ts (1)
1-59
: 🛠️ Refactor suggestionAdd type export for the database schema
Unlike the
kubernetes-namespace.ts
file which exports its type, this file doesn't export a TypeScript type for the database schema. Consider adding:export type Database = z.infer<typeof database>;This would provide consistent typing across your resource kinds and make it easier for consumers to use the schema types.
♻️ Duplicate comments (1)
packages/validators/src/resources/kinds/storage-v1/database.ts (1)
14-48
: Enforce strong passwords and support secret references in storage auth schemasTo improve security, let's enhance all
password
fields inauthMethod
:
- Define a reusable
passwordSchema
, e.g.:const passwordSchema = z.union([ z.string() .min(8) .refine(val => /[A-Z]/.test(val) && /[a-z]/.test(val) && /[0-9]/.test(val), { message: "Must be ≥8 chars with upper, lower & number", }), z.object({ secretRef: z.string() }), // allow vault references ]);- Apply it across all methods (mandatory or
.optional()
):z.object({ type: z.literal("direct"), host: z.string(), port: z.number(), username: z.string(),- password: z.string(),
- password: passwordSchema,
ssl: z.boolean().optional(),
}),
// similarly update aws, google, azure: password: passwordSchema.optional()- Add a note in your docs/README on secure storage and handling of credentials (e.g., environment vars, vault). This both enforces strength rules and accommodates secret‐management integrations. </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts (1)</summary><blockquote> `6-29`: **Consider improving the type definitions for Kubernetes metadata** The schema is well-structured, but there are opportunities for improvement: 1. The `"kubernetes/autoscaling-enabled"` field is defined as a string (line 25), but it seems to represent a boolean value. Consider using `z.boolean()` instead. 2. For better maintainability, the status literals could be extracted to a constant: ```typescript const K8S_STATUS = ['running', 'unknown', 'creating', 'deleting'] as const; // ... "kubernetes/status": z.enum(K8S_STATUS),
- Version fields (lines 21-24) could benefit from regex validation to ensure they follow semantic versioning format.
packages/validators/src/resources/kinds/storage-v1/database.ts (2)
5-7
: Add more comprehensive documentation for the database resourceThe comment provides a good start on operations, but consider expanding it to include:
- Examples of supported database engines
- Version format expectations
- Required permissions for different auth methods
This would help users understand how to correctly configure database resources.
50-58
: Consider adding enumerated values for database statusThe
"database/status"
field allows any string value. For better type safety and validation, consider using an enumeration of known database statuses:metadata: z .object({ "database/engine": z.string(), "database/version": z.string(), - "database/status": z.string(), + "database/status": z.enum(["available", "creating", "deleting", "failed", "maintenance", "updating"]), }) .partial() .passthrough(),This would provide better validation and documentation of valid status values.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/machine.ts
(1 hunks)packages/validators/src/resources/kinds/create-kind.ts
(1 hunks)packages/validators/src/resources/kinds/network-v1/network.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/bucket.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/database.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/file-system.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/validators/src/resources/kinds/compute-v1/machine.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/validators/src/resources/kinds/storage-v1/file-system.ts
- packages/validators/src/resources/kinds/network-v1/network.ts
- packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
- packages/validators/src/resources/kinds/storage-v1/bucket.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}
: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/validators/src/resources/kinds/create-kind.ts
packages/validators/src/resources/kinds/compute-v1/kubernetes-namespace.ts
packages/validators/src/resources/kinds/storage-v1/database.ts
🧬 Code Graph Analysis (1)
packages/validators/src/resources/kinds/storage-v1/database.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-22)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
3-22
: Well-designed utility function for consistent resource schema creationThe
createKind
function provides a clean abstraction for creating standardized resource kind schemas. It properly uses generics to maintain type safety and ensures a consistent structure across all resources with required fields (identifier
,name
), literal values forversion
andkind
, and properly typedconfig
andmetadata
objects.This approach will help enforce consistency across all resource kinds in the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/validators/src/resources/kinds/storage-v1/bucket.ts (1)
13-42
: Secure raw credential fields in provider configurationsThe provider schemas include raw credential fields (accessKey, secretKey, serviceAccountKey) without any format validation or secure handling best practices.
Consider:
- Using references to secrets stored in a secure vault instead of raw literals
- Adding format validations for credential fields
- Documenting secure handling recommendations
For example:
z.object({ type: z.literal("aws"), accountId: z.string(), region: z.string(), - accessKey: z.string().optional(), - secretKey: z.string().optional(), + accessKey: z.string() + .regex(/^[A-Z0-9]{20}$/, { + message: "AWS Access Key must be 20 uppercase alphanumeric characters" + }) + .optional() + .describe("Reference to securely stored credential"), + secretKey: z.string() + .min(40) + .optional() + .describe("Reference to securely stored credential"), endpoint: z.string().url().optional(), }),Apply similar patterns to Google, Azure, and S3-compatible provider schemas.
🧹 Nitpick comments (2)
packages/validators/src/resources/kinds/storage-v1/bucket.ts (2)
53-63
: Consider removing sensitive metadata fieldsThe metadata schema appears to store bucket information but might be a location where sensitive information gets accidentally logged.
Consider:
- Being explicit about which fields should never contain sensitive data
- Adding description annotations to warn against storing credentials in metadata
- Adding a validation schema for encryption status that only accepts safe values
For example:
metadata: z .object({ "bucket/version": z.string(), "bucket/status": z.string(), "bucket/type": z.string(), "bucket/region": z.string(), - "bucket/encryption": z.string(), + "bucket/encryption": z.enum(["enabled", "disabled"]) + .describe("Encryption status - do not include encryption keys"), }) .partial() .passthrough(),
1-63
: Add TypeScript type exports for improved developer experienceThe validator schema is defined but no TypeScript types are exported, which would be helpful for consumers of this library.
Consider adding type exports like:
export type Bucket = z.infer<typeof bucket>; export type BucketConfig = z.infer<typeof bucket>["config"]; export type BucketProvider = z.infer<typeof bucket>["config"]["provider"];This will allow consumers to use these types in their TypeScript code without having to extract them from the schema.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
(1 hunks)packages/validators/src/resources/kinds/compute-v1/machine.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/bucket.ts
(1 hunks)packages/validators/src/resources/kinds/storage-v1/database.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/validators/src/resources/kinds/compute-v1/kubernetes-cluster.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/validators/src/resources/kinds/compute-v1/machine.ts
- packages/validators/src/resources/kinds/storage-v1/database.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}
: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/validators/src/resources/kinds/storage-v1/bucket.ts
🧬 Code Graph Analysis (1)
packages/validators/src/resources/kinds/storage-v1/bucket.ts (1)
packages/validators/src/resources/kinds/create-kind.ts (1)
createKind
(3-22)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (4)
packages/validators/src/resources/kinds/storage-v1/bucket.ts (4)
1-4
: Clean imports with appropriate separationThe imports are well-organized with clear separation between external and internal dependencies.
5-9
: Good documentation and resource definitionThe comment clearly explains the purpose of this resource (file storage, backup, static hosting), and the schema utilizes the
createKind
helper appropriately with the correct version and kind.
9-12
: Core configuration properties look goodThe base configuration with required
name
andregion
fields is appropriate for bucket resources across different providers.
44-52
: Well-structured encryption configurationThe encryption configuration properly handles different encryption types with appropriate optional fields. The support for multiple encryption standards (SSE-S3, SSE-KMS, SSE-C) provides good flexibility.
Summary by CodeRabbit