EDM-2913: Add support for OCI repositories #449
Conversation
WalkthroughAdds OCI registry support and related types/exports, extends RepositorySpec and form/schema/validation/patch logic for OCI, updates UI across repository create/list/details/import flows, tightens repository selection behavior, and adds English i18n keys for OCI fields and validation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (13)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-10T11:35:04.458ZApplied to files:
📚 Learning: 2025-11-20T09:24:41.991ZApplied to files:
🧬 Code graph analysis (4)libs/ui-components/src/components/Repository/CreateRepository/utils.ts (7)
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (4)
libs/types/models/OciRepoSpec.ts (2)
libs/types/models/RepositorySpec.ts (5)
🪛 Biome (2.1.2)libs/ui-components/src/components/Repository/CreateRepository/utils.ts[error] 626-626: Do not add then to an object. (lint/suspicious/noThenProperty) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (16)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/types/models/K8sProviderSpec.ts (1)
1-37: Critical: Manual edits to generated code will be overwritten.This file is auto-generated from an OpenAPI specification (line 1: "do no edit"). Any manual changes made here will be lost when the code is regenerated.
The
roleSuffixfield addition should instead be made in the source OpenAPI/Swagger specification file, then regenerated usingopenapi-typescript-codegen.
Note: The field definition itself appears correct and well-documented, but it must originate from the API specification to persist.
libs/types/models/OpenShiftProviderSpec.ts (1)
1-57: Critical: Manual edits to generated code will be overwritten.This file is auto-generated from an OpenAPI specification (line 1: "do no edit"). Any manual changes made here will be lost when the code is regenerated.
Both the
projectLabelFilterandroleSuffixfield additions should instead be made in the source OpenAPI/Swagger specification file, then regenerated usingopenapi-typescript-codegen.
Note: The field definitions themselves appear correct and well-documented:
projectLabelFilterprovides clear label selector format guidanceroleSuffixmaintains consistency with the identical field inK8sProviderSpec.tsHowever, they must originate from the API specification to persist.
🧹 Nitpick comments (4)
libs/i18n/locales/en/translation.json (1)
1004-1005: Minor inconsistency in capitalization.Line 1005 uses
"Url"while the rest of the file consistently uses"URL"(e.g., lines 64-67, 686, 956). Consider updating for consistency.- "Url": "Url", + "Url": "URL",libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (1)
194-205: Use enum values instead of string literals for consistency.The default values use string literals (
'https','Read') while the RadioField components use enum values (OciRepoSpec.scheme.HTTPS,OciRepoSpec.accessMode.READ). For consistency and type safety, use the enum values here as well.🔎 Proposed fix
if (toType === RepoSpecType.OCI) { void setFieldValue('repoType', RepoSpecType.OCI); void setFieldValue('useResourceSyncs', false); void setFieldValue('canUseResourceSyncs', false); if (!values.ociConfig) { void setFieldValue('ociConfig', { registry: '', - scheme: 'https', - accessMode: 'Read', + scheme: OciRepoSpec.scheme.HTTPS, + accessMode: OciRepoSpec.accessMode.READ, }); } }libs/ui-components/src/components/Repository/CreateRepository/utils.ts (2)
593-596: Registry hostname regex allows some invalid IP addresses.The IPv4 validation accepts octets > 255 (e.g.,
999.999.999.999). For UI validation this is acceptable since the backend should perform stricter validation, but consider tightening if stricter client-side validation is desired.
285-302: Simplify the OCI type check.The type check using
isOciRepoSpec({ type: values.repoType } as RepositorySpec)is unnecessarily complex. A direct comparison is clearer.🔎 Proposed fix
- if (isOciRepoSpec(repository.spec) && !isOciRepoSpec({ type: values.repoType } as RepositorySpec)) { + if (isOciRepoSpec(repository.spec) && values.repoType !== RepoSpecType.OCI) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
libs/i18n/locales/en/translation.jsonlibs/types/index.tslibs/types/models/ApplicationVolume.tslibs/types/models/ApplicationVolumeReclaimPolicy.tslibs/types/models/DockerAuth.tslibs/types/models/K8sProviderSpec.tslibs/types/models/OciAuth.tslibs/types/models/OciAuthType.tslibs/types/models/OciRepoSpec.tslibs/types/models/OpenShiftProviderSpec.tslibs/types/models/RepoSpecType.tslibs/types/models/RepositorySpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsxlibs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsxlibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsxlibs/ui-components/src/components/Repository/CreateRepository/types.tslibs/ui-components/src/components/Repository/CreateRepository/utils.tslibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/form/FormSelect.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to handle undefined input through its optional parameter definition and consistent use of optional chaining.
Applied to files:
libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to safely handle undefined values for the updatePolicy parameter.
Applied to files:
libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx
📚 Learning: 2025-10-10T11:35:04.458Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-10-10T11:35:04.458Z
Learning: In the flightctl-ui repository, TypeScript types are generated using hey-api/openapi-ts and output to a single index.ts file (previously used openapi-typescript-codegen with one file per type under /libs/types/models).
Applied to files:
libs/ui-components/src/components/Repository/CreateRepository/types.tslibs/types/models/OciAuth.tslibs/types/index.ts
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/types/models/OciAuth.tslibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsxlibs/types/index.ts
📚 Learning: 2025-02-17T08:41:57.993Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-02-17T08:41:57.993Z
Learning: All files in `libs/types/models` are auto-generated using OpenAPI Generator from the schema defined in https://github.com/flightctl/flightctl/blob/main/api/v1alpha1/openapi.yaml. Changes to these files should be made in the schema instead of modifying the generated files directly.
Applied to files:
libs/types/index.ts
🧬 Code graph analysis (13)
libs/types/models/OciAuthType.ts (1)
libs/types/index.ts (1)
OciAuthType(148-148)
libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx (2)
libs/types/index.ts (1)
GenericRepoSpec(113-113)libs/types/models/GenericRepoSpec.ts (1)
GenericRepoSpec(6-12)
libs/types/models/ApplicationVolumeReclaimPolicy.ts (1)
libs/types/index.ts (1)
ApplicationVolumeReclaimPolicy(18-18)
libs/ui-components/src/components/Repository/RepositoryList.tsx (2)
libs/types/index.ts (1)
RepoSpecType(165-165)libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
libs/types/models/DockerAuth.ts (1)
libs/types/index.ts (1)
DockerAuth(81-81)
libs/ui-components/src/components/Repository/CreateRepository/types.ts (2)
libs/types/index.ts (1)
OciRepoSpec(149-149)libs/types/models/OciRepoSpec.ts (1)
OciRepoSpec(10-33)
libs/types/models/OciRepoSpec.ts (2)
libs/types/index.ts (3)
OciRepoSpec(149-149)RepoSpecType(165-165)OciAuth(147-147)libs/types/models/OciAuth.ts (1)
OciAuth(9-9)
libs/types/models/OciAuth.ts (2)
libs/types/index.ts (2)
OciAuth(147-147)DockerAuth(81-81)libs/types/models/DockerAuth.ts (1)
DockerAuth(8-18)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (3)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (2)
isOciRepoSpec(35-35)getRepoUrlOrRegistry(37-42)libs/types/models/Repository.ts (1)
Repository(11-23)libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySource.tsx (2)
HttpRepositoryUrl(34-42)GitRepositoryLink(44-56)
libs/types/models/ApplicationVolume.ts (1)
libs/types/index.ts (1)
ApplicationVolumeReclaimPolicy(18-18)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (3)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)libs/types/index.ts (2)
GenericRepoSpec(113-113)HttpRepoSpec(123-123)libs/ui-components/src/types/deviceSpec.ts (1)
HttpConfigTemplate(208-214)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (7)
libs/types/index.ts (5)
RepositorySpec(163-163)OciRepoSpec(149-149)RepoSpecType(165-165)DockerAuth(81-81)Repository(161-161)libs/types/models/RepositorySpec.ts (1)
RepositorySpec(12-12)libs/types/models/OciRepoSpec.ts (1)
OciRepoSpec(10-33)libs/ui-components/src/components/Repository/CreateRepository/types.ts (1)
RepositoryFormValues(10-55)libs/ui-components/src/utils/patch.ts (1)
appendJSONPatch(23-54)libs/types/models/DockerAuth.ts (1)
DockerAuth(8-18)libs/types/models/Repository.ts (1)
Repository(11-23)
🪛 Biome (2.1.2)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts
[error] 649-649: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Build ocp plugin
🔇 Additional comments (26)
libs/ui-components/src/components/form/FormSelect.tsx (1)
9-9: Code change is type-safe and compatible with PatternFly.The type widening from
stringtostring | React.ReactNodefor thedescriptionproperty is verified as compatible. PatternFly'sSelectOptioncomponent acceptsReact.ReactNodefor its description prop, confirming the change enables richer descriptions for OCI repository configurations without introducing type mismatches.libs/types/models/OciAuthType.ts (1)
1-10: LGTM!Auto-generated enum for OCI authentication types. The single
DOCKERmember aligns with Docker-based registry authentication, which is the standard for OCI registries.libs/types/models/ApplicationVolumeReclaimPolicy.ts (1)
1-10: LGTM!Auto-generated enum for volume reclaim policies. The
RETAINpolicy follows Kubernetes conventions for volume lifecycle management.libs/types/models/ApplicationVolume.ts (1)
5-15: LGTM!The new optional
reclaimPolicyproperty is correctly typed and follows the established pattern for auto-generated types. The type-only import is appropriate.libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx (1)
38-63: LGTM!Good type safety improvement. The comment on line 39 clarifies that only Git repositories are used for importing fleets, making the type assertion appropriate. The optional chaining with fallback on line 63 handles the undefined case gracefully.
libs/types/models/RepoSpecType.ts (1)
8-12: LGTM!The new
OCIenum member follows the existing naming convention with lowercase values. This is the foundational type change for OCI repository support.libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
10-10: LGTM!Good use of the
getRepoUrlOrRegistryutility to handle both OCI registries and non-OCI repository URLs consistently. This centralizes the extraction logic and ensures proper handling of the new OCI repository type.Also applies to: 49-49
libs/ui-components/src/components/Repository/RepositoryList.tsx (1)
36-36: LGTM! Clean OCI repository support integration.The changes properly handle OCI registry display with:
- Type-specific labels ("OCI registry", "HTTP service", "Git repository")
- Dynamic column labels ("Registry" vs "Url")
- Proper use of the
getRepoUrlOrRegistryhelper with fallback handlingAlso applies to: 137-145
libs/types/models/OciAuth.ts (1)
1-10: LGTM! Auto-generated type definition.The OciAuth type alias properly reuses DockerAuth for OCI registry authentication. As this is an auto-generated file from the OpenAPI schema, the structure is correct.
Based on learnings, this file is auto-generated from the OpenAPI schema.
libs/types/models/DockerAuth.ts (1)
1-19: LGTM! Well-structured authentication type.The DockerAuth type properly defines Docker-style authentication with:
- Discriminator field (
authType: 'docker') for type safety- Required username and password fields
- Clear JSDoc documentation
Based on learnings, this file is auto-generated from the OpenAPI schema.
libs/types/models/RepositorySpec.ts (1)
7-7: LGTM! Proper extension of repository types.The RepositorySpec union type correctly includes OciRepoSpec, enabling OCI repository support alongside existing Git, HTTP, and SSH types.
Based on learnings, this file is auto-generated from the OpenAPI schema.
Also applies to: 12-12
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (3)
20-20: LGTM! Proper OCI authentication privacy detection.The privacy check correctly identifies OCI repositories with authentication as private, consistent with the existing logic for HTTP and SSH repositories.
Also applies to: 34-37
57-66: LGTM! Clean component for URL/registry rendering.The
RegistryOrUrlcomponent properly encapsulates the rendering logic:
- OCI repos display plain registry text
- HTTP repos use
HttpRepositoryUrlwith copy functionality- Git repos use
GitRepositoryLinkwith external link
71-93: LGTM! Clear type-based labeling and rendering.The repository type label computation and dynamic field labels properly distinguish between OCI registries, HTTP services, and Git repositories, providing a consistent user experience.
libs/types/index.ts (1)
18-18: LGTM! Proper public API exports for OCI support.The new exports correctly extend the public API surface with OCI-related types (DockerAuth, OciAuth, OciAuthType, OciRepoSpec) following the existing export patterns.
Note: The
ApplicationVolumeReclaimPolicyexport on line 18 appears unrelated to OCI repository support and may be part of a separate feature addition.Based on learnings, these exports are auto-generated from the OpenAPI schema.
Also applies to: 81-81, 147-149
libs/ui-components/src/components/Repository/CreateRepository/types.ts (1)
1-1: LGTM! Consistent OCI configuration structure.The
ociConfigaddition toRepositoryFormValuesfollows the established pattern ofhttpConfigandsshConfig:
- Registry field for OCI-specific endpoint
- Properly typed
schemeandaccessModeusingOciRepoSpecnamespace types- Authentication structure mirrors
basicAuthpattern- Includes standard security options (caCrt, skipServerVerification)
The form field naming (
caCrt) vs. the model naming (ca.crt) difference is appropriate for form handling.Also applies to: 40-51
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx (1)
96-96: This is a new file created in the OCI repository support commit. The conditionrepoDetails.spec.type === RepoSpecType.GITwas not narrowed from a previous state—there was no prior condition. Additionally, SSH repositories are not excluded; they are configured as GIT-type repositories (type: RepoSpecType.GIT) with SSH transport, as evidenced by the SshRepoSpec definition. The condition correctly enables ResourceSyncsCard for all GIT-type repositories, including those using SSH.libs/types/models/OciRepoSpec.ts (1)
1-50: LGTM - Auto-generated type definition for OCI repository specification.The type structure is well-defined with appropriate fields for OCI registry configuration including authentication, TLS settings, and access modes.
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (3)
51-88: LGTM - OCI advanced configuration section.The OCI-specific advanced section correctly handles basic authentication, server verification skip, and CA certificate fields. The CA certificate field is appropriately disabled when server verification is skipped.
248-258: LGTM - OCI radio button option.The new OCI registry radio option follows the same pattern as the existing Git and HTTP options, maintaining UI consistency.
309-371: LGTM - OCI-specific form fields.The conditional rendering for OCI registry hostname (vs. URL for Git/HTTP) and the scheme/access mode configuration sections are well-structured with appropriate labels and helper text.
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (5)
35-42: LGTM - Type guard and helper for OCI repositories.The
isOciRepoSpectype guard correctly identifies OCI repositories by checking the type field, andgetRepoUrlOrRegistryprovides a clean abstraction for accessing the appropriate identifier field.
647-663: Static analysis false positive - Yup conditional validation pattern is correct.The Biome warning about
thenis a false positive. This is Yup's standard API for conditional schema validation using.when()withthen/otherwisecallbacks, not adding athenproperty to create a thenable object.
667-700: LGTM - OCI repository construction.The OCI repository spec is correctly built with proper handling of optional fields (CA certificate, authentication) and appropriate defaults for scheme and access mode.
120-141: LGTM - OCI configuration initialization from existing repository.The form values are correctly populated from an existing OCI repository spec, including proper base64 decoding of the CA certificate.
621-643: LGTM - OCI validation schema.The OCI-specific validation correctly requires the registry hostname and conditionally validates authentication credentials based on the
useflag.
...components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx
Show resolved
Hide resolved
celdrake
force-pushed
the
EDM-2913-support-oci-repositories
branch
from
2469233 to
694cc4c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
209-218: Logic error: HttpConfigForm renders for non-HTTP repository types.The condition on line 212 (
repoSpec &&) will renderHttpConfigFormwhenever a repository is selected, regardless of its type. This causes Git repositories to incorrectly display the HTTP configuration form.The fix should restore the type check:
🔎 Proposed fix
{repoType === RepoSpecType.GIT && ( <GitConfigForm template={ct as GitConfigTemplate} index={index} isReadOnly={isReadOnly} /> )} - {repoSpec && ( + {repoType === RepoSpecType.HTTP && repoSpec && ( <HttpConfigForm template={ct as HttpConfigTemplate} index={index} baseURL={repoSpec.url || ''} isReadOnly={isReadOnly} /> )}
🧹 Nitpick comments (2)
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (1)
187-213: Consider initializing URL when switching from OCI to Git/HTTP.When switching from OCI to Git or HTTP, the
urlfield might be empty (since OCI usesociConfig.registryinstead). Consider clearing or initializingurlto avoid form validation issues.🔎 Suggested enhancement
if (toType === RepoSpecType.GIT) { void setFieldValue('repoType', RepoSpecType.GIT); void setFieldValue('httpConfig.token', undefined); void setFieldValue('canUseResourceSyncs', true); + // Ensure url is available when switching from OCI + if (values.repoType === RepoSpecType.OCI) { + void setFieldValue('url', ''); + } } if (toType === RepoSpecType.HTTP) { void setFieldValue('repoType', RepoSpecType.HTTP); void setFieldValue('configType', 'http'); void setFieldValue('useResourceSyncs', false); void setFieldValue('canUseResourceSyncs', false); + // Ensure url is available when switching from OCI + if (values.repoType === RepoSpecType.OCI) { + void setFieldValue('url', ''); + } }libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
285-302: Consider simplifying the type check for switching away from OCI.The expression
isOciRepoSpec({ type: values.repoType } as RepositorySpec)constructs a partial object just to check the type. A direct comparison would be clearer.🔎 Suggested simplification
- if (isOciRepoSpec(repository.spec) && !isOciRepoSpec({ type: values.repoType } as RepositorySpec)) { + if (isOciRepoSpec(repository.spec) && values.repoType !== RepoSpecType.OCI) { patches.push({ op: 'remove', path: '/spec/registry' });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
libs/i18n/locales/en/translation.jsonlibs/types/index.tslibs/types/models/ApplicationVolume.tslibs/types/models/ApplicationVolumeReclaimPolicy.tslibs/types/models/DockerAuth.tslibs/types/models/K8sProviderSpec.tslibs/types/models/OciAuth.tslibs/types/models/OciAuthType.tslibs/types/models/OciRepoSpec.tslibs/types/models/OpenShiftProviderSpec.tslibs/types/models/RepoSpecType.tslibs/types/models/RepositorySpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsxlibs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsxlibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsxlibs/ui-components/src/components/Repository/CreateRepository/types.tslibs/ui-components/src/components/Repository/CreateRepository/utils.tslibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/form/FormSelect.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- libs/ui-components/src/components/form/FormSelect.tsx
- libs/types/models/OciAuthType.ts
- libs/types/index.ts
- libs/types/models/ApplicationVolumeReclaimPolicy.ts
- libs/types/models/RepoSpecType.ts
- libs/types/models/DockerAuth.ts
- libs/types/models/OciRepoSpec.ts
- libs/i18n/locales/en/translation.json
- libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx
- libs/types/models/RepositorySpec.ts
- libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx
- libs/types/models/OciAuth.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-10T11:35:04.458Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-10-10T11:35:04.458Z
Learning: In the flightctl-ui repository, TypeScript types are generated using hey-api/openapi-ts and output to a single index.ts file (previously used openapi-typescript-codegen with one file per type under /libs/types/models).
Applied to files:
libs/ui-components/src/components/Repository/CreateRepository/types.ts
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx
🧬 Code graph analysis (4)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (4)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (3)
isOciRepoSpec(35-35)getRepoUrlOrRegistry(37-42)isHttpRepoSpec(32-33)libs/types/models/Repository.ts (1)
Repository(11-23)libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySource.tsx (2)
HttpRepositoryUrl(34-42)GitRepositoryLink(44-56)libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
libs/ui-components/src/components/Repository/RepositoryList.tsx (2)
libs/types/index.ts (1)
RepoSpecType(165-165)libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
libs/types/models/ApplicationVolume.ts (1)
libs/types/index.ts (1)
ApplicationVolumeReclaimPolicy(18-18)
🪛 Biome (2.1.2)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts
[error] 649-649: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Build ocp plugin
- GitHub Check: Build
- GitHub Check: Lint
🔇 Additional comments (17)
libs/types/models/K8sProviderSpec.ts (1)
1-37: Inconsistency: This file does not relate to OCI repository support.The PR objectives state this PR "adds support for OCI repositories," but this file modifies a Kubernetes authentication provider spec (
K8sProviderSpec), adding aroleSuffixfield for ClusterRole name normalization. This appears unrelated to OCI repository functionality.Additionally, this is a generated file (line 1: "generated using openapi-typescript-codegen -- do no edit"). Any changes should be made to the source OpenAPI specification, not directly to this TypeScript file. If this file was regenerated as part of an API spec update, verify that the spec changes are intentional and properly documented.
Please confirm:
- Whether this file should be part of PR #449 (OCI repository support)
- Whether the source OpenAPI specification was updated to include these provider spec changes
- If these authentication provider changes are related to OCI support, or if they belong in a separate PR
libs/types/models/OpenShiftProviderSpec.ts (1)
1-57: Inconsistency: This file does not relate to OCI repository support.Similar to
K8sProviderSpec.ts, this file modifies an OpenShift authentication provider spec by addingprojectLabelFilterandroleSuffixfields. These changes appear unrelated to the PR's stated objective of adding OCI repository support.This is also a generated file (line 1: "generated using openapi-typescript-codegen -- do no edit"), so changes should originate from the source OpenAPI specification.
The field additions themselves are reasonable:
projectLabelFilterenables server-side filtering of OpenShift projects by label (OpenShift-specific feature)roleSuffixmaintains consistency with the K8sProviderSpec for multi-release deploymentsHowever, please verify that these authentication provider changes were intended for this PR, or if they were inadvertently included during code generation.
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
34-38: LGTM on repository item description handling.The use of
getRepoUrlOrRegistryto derive descriptions is a good improvement that handles OCI registries consistently with other repository types.libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
10-10: LGTM on centralized URL/registry extraction.Using
getRepoUrlOrRegistryprovides consistent handling across OCI, HTTP, and Git repository types.Also applies to: 49-49
libs/ui-components/src/components/Repository/RepositoryList.tsx (1)
137-145: LGTM on OCI type handling in repository list.The type display and dynamic URL/Registry label provide clear differentiation for OCI repositories. The use of
getRepoUrlOrRegistryensures consistent rendering.libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (2)
34-37: LGTM on OCI privacy detection.Correctly identifies OCI repositories as private when
ociAuthis present, consistent with HTTP/SSH authentication checks.
57-66: LGTM on RegistryOrUrl component.Clean extraction that routes rendering based on repository type. OCI shows plain text, HTTP includes copy functionality, and Git provides an external link.
libs/ui-components/src/components/Repository/CreateRepository/types.ts (1)
40-51: LGTM on OCI configuration type definition.The
ociConfigstructure follows the established pattern fromhttpConfigandsshConfig, with appropriate fields for registry authentication and TLS configuration.libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (2)
55-88: LGTM on OCI-specific AdvancedSection.The OCI authentication and TLS configuration fields follow the same patterns as HTTP configuration, providing a consistent user experience.
311-327: LGTM on conditional OCI registry field.Good conditional rendering that shows the appropriate input field based on repository type.
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (5)
35-42: LGTM on OCI utility functions.Clean type guard and URL/registry extraction helper. These are well-designed for reuse across the codebase.
182-274: LGTM on OCI patch generation.Comprehensive handling of OCI-specific fields including registry, scheme, accessMode, authentication, and CA certificate. Properly cleans up HTTP/SSH config when switching to OCI.
593-595: Registry hostname regex looks comprehensive.The regex handles FQDN, IPv4, and IPv6 addresses with optional port numbers, which covers common OCI registry endpoint formats.
647-663: Static analysis false positive - Yup.when()syntax is valid.The Biome warning about
thenproperty is a false positive. Yup's.when()method usesthenas a callback function parameter, which is the standard API pattern for conditional validation in Yup schemas.
668-700: LGTM on OCI repository construction.The
getRepositoryfunction correctly builds a completeOciRepoSpecwith all optional fields (skipServerVerification, CA certificate, ociAuth) based on form values.libs/types/models/ApplicationVolume.ts (2)
14-14: No issues found. TheApplicationVolumeReclaimPolicytype is properly defined as an enum with theRETAINvalue, correctly imported, and the optionalreclaimPolicyfield is appropriately integrated into theApplicationVolumetype for backward compatibility.
1-5: The files are properly auto-generated and include the correct generation markers. BothApplicationVolume.tsand the newApplicationVolumeReclaimPolicy.tsshow the expected "generated using openapi-typescript-codegen -- do no edit" header, and the generation workflow vianpm run gen-typesexists in the repository configuration. If the external OpenAPI schema in the flightctl repository was updated to include theApplicationVolumeReclaimPolicyfield for OCI volume support, regenerating types through the standard workflow produces these expected changes.Likely an incorrect or invalid review comment.
celdrake
force-pushed
the
EDM-2913-support-oci-repositories
branch
from
694cc4c to
58bd472
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
209-219: Logic error: HttpConfigForm renders for non-HTTP repository types.The condition on line 212 (
repoSpec &&) rendersHttpConfigFormwheneverrepoSpecexists. This incorrectly displays the HTTP configuration form for GIT repositories.🔎 Proposed fix
{repoType === RepoSpecType.GIT && ( <GitConfigForm template={ct as GitConfigTemplate} index={index} isReadOnly={isReadOnly} /> )} - {repoSpec && ( + {repoType === RepoSpecType.HTTP && repoSpec && ( <HttpConfigForm template={ct as HttpConfigTemplate} index={index} baseURL={repoSpec.url || ''} isReadOnly={isReadOnly} /> )}
🧹 Nitpick comments (3)
libs/i18n/locales/en/translation.json (1)
1005-1005: Inconsistent capitalization: "Url" should be "URL".The acronym "URL" should use all uppercase letters for consistency with other keys in this file (e.g., line 686
"URL": "URL"and line 956"Repository URL": "Repository URL").Suggested fix
- "Url": "Url", + "URL": "URL",Note: This change would also require updating the corresponding usage in the UI components to reference the new key name.
libs/ui-components/src/components/Repository/RepositoryList.tsx (1)
71-87: Consider: Static column header vs dynamic data label.The column header at line 79 is always
t('Url'), but thedataLabelat line 143 dynamically switches between'Registry'and'Url'based on repository type. This inconsistency may be intentional for table uniformity, but could also be confusing when viewing OCI repositories.Consider making the column header generic (e.g., "Source" or "Location") to accommodate both URL and Registry semantics, or keep it as-is if the current behavior is acceptable.
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (1)
71-78: Consider extracting shared repository label utility.The
repoLabelcomputation here duplicates the logic inRepositoryList.tsx(lines 137-141). Consider extracting this to a shared utility function inCreateRepository/utils.tsto ensure consistency and reduce duplication.🔎 Proposed utility extraction
// In CreateRepository/utils.ts export const getRepoTypeLabel = (repoSpec: RepositorySpec, t: TFunction): string => { if (isOciRepoSpec(repoSpec)) { return t('OCI registry'); } if (isHttpRepoSpec(repoSpec)) { return t('HTTP service'); } return t('Git repository'); };Then use in both files:
const repoLabel = getRepoTypeLabel(repoDetails.spec, t);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
libs/i18n/locales/en/translation.jsonlibs/types/index.tslibs/types/models/ApplicationVolume.tslibs/types/models/ApplicationVolumeReclaimPolicy.tslibs/types/models/DockerAuth.tslibs/types/models/K8sProviderSpec.tslibs/types/models/OciAuth.tslibs/types/models/OciAuthType.tslibs/types/models/OciRepoSpec.tslibs/types/models/OpenShiftProviderSpec.tslibs/types/models/RepoSpecType.tslibs/types/models/RepositorySpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsxlibs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsxlibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsxlibs/ui-components/src/components/Repository/CreateRepository/types.tslibs/ui-components/src/components/Repository/CreateRepository/utils.tslibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/form/FormSelect.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/types/models/RepoSpecType.ts
- libs/types/models/RepositorySpec.ts
- libs/ui-components/src/components/form/FormSelect.tsx
- libs/types/models/ApplicationVolumeReclaimPolicy.ts
- libs/types/models/OciAuth.ts
- libs/types/models/OciAuthType.ts
- libs/types/models/ApplicationVolume.ts
- libs/types/models/OpenShiftProviderSpec.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-02-04T09:04:36.106Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 207
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewDeviceStep.tsx:55-55
Timestamp: 2025-02-04T09:04:36.106Z
Learning: The codebase has automated CI checks ("lint" step) that verify translation coverage for all text strings, ensuring that new or modified strings have corresponding entries in translation files.
Applied to files:
libs/i18n/locales/en/translation.json
📚 Learning: 2025-10-10T11:35:04.458Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-10-10T11:35:04.458Z
Learning: In the flightctl-ui repository, TypeScript types are generated using hey-api/openapi-ts and output to a single index.ts file (previously used openapi-typescript-codegen with one file per type under /libs/types/models).
Applied to files:
libs/types/index.tslibs/types/models/OciRepoSpec.tslibs/ui-components/src/components/Repository/CreateRepository/types.ts
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/types/index.tslibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx
🧬 Code graph analysis (6)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx (1)
libs/types/models/GenericRepoSpec.ts (1)
GenericRepoSpec(6-12)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (3)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)libs/types/index.ts (2)
GenericRepoSpec(113-113)HttpRepoSpec(123-123)libs/ui-components/src/types/deviceSpec.ts (1)
HttpConfigTemplate(208-214)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx (1)
libs/types/index.ts (1)
RepoSpecType(165-165)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (5)
libs/types/index.ts (7)
RepositorySpec(163-163)OciRepoSpec(149-149)RepoSpecType(165-165)PatchRequest(155-155)DockerAuth(81-81)OciAuthType(148-148)Repository(161-161)libs/types/models/RepositorySpec.ts (1)
RepositorySpec(12-12)libs/types/models/OciRepoSpec.ts (1)
OciRepoSpec(10-33)libs/types/models/DockerAuth.ts (1)
DockerAuth(8-18)libs/types/models/Repository.ts (1)
Repository(11-23)
libs/types/models/OciRepoSpec.ts (1)
libs/types/models/OciAuth.ts (1)
OciAuth(9-9)
🪛 Biome (2.1.2)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts
[error] 626-626: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration-tests
🔇 Additional comments (29)
libs/i18n/locales/en/translation.json (3)
931-935: LGTM!Standard authentication-related labels are correctly added for OCI registry support.
950-962: LGTM!OCI registry configuration labels are well-structured with helpful examples for the registry hostname field.
980-985: LGTM!Validation messages are clear and include helpful examples to guide users.
libs/types/models/K8sProviderSpec.ts (1)
33-36: No issues found. TheroleSuffixproperty is properly integrated as part of the auto-generated code from the upstream OpenAPI specification (https://raw.githubusercontent.com/flightctl/flightctl/main/api/v1beta1/openapi.yaml in the flightctl repository). The consistent presence of this property across bothK8sProviderSpec.tsandOpenShiftProviderSpec.tswith identical documentation confirms it was added to the OpenAPI spec and the TypeScript types were correctly regenerated, not manually edited.libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
10-10: LGTM!The integration of
getRepoUrlOrRegistrycorrectly abstracts the URL/registry retrieval logic, enabling seamless support for OCI repositories alongside Git and HTTP repositories.Also applies to: 49-49
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx (1)
96-102: LGTM!Appropriately restricts the
RepositoryResourceSyncsCardto Git repositories only, as ResourceSyncs are not applicable to OCI or HTTP repository types.libs/ui-components/src/components/Repository/RepositoryList.tsx (1)
137-145: LGTM!The type display logic correctly differentiates between OCI registries, HTTP services, and Git repositories. The dynamic URL/Registry label and the use of
getRepoUrlOrRegistryensure consistent rendering across all repository types.libs/types/models/DockerAuth.ts (1)
1-18: LGTM!This is an auto-generated type definition from the OpenAPI schema. The
DockerAuthtype correctly models Docker-style authentication with a discriminantauthType: 'docker'for type narrowing in union contexts.libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx (2)
63-63: LGTM on the URL access pattern.Using optional chaining with
repoSpec?.url || '-'safely handles cases where the spec or URL might be undefined.
38-39: The repositories passed toRepositoryStepare already filtered to Git-only repositories in the parent component (ImportFleetWizard.tsxline 107:filter((repo) => repo.spec.type === RepoSpecType.GIT)), so the type assertion toGenericRepoSpec | undefinedis safe and valid. No OCI repositories can reach this component.Likely an incorrect or invalid review comment.
libs/types/index.ts (1)
18-18: LGTM!The new type exports (
ApplicationVolumeReclaimPolicy,DockerAuth,OciAuth,OciAuthType,OciRepoSpec) are correctly added to the public API surface. The export syntax appropriately distinguishes between types and enum/namespace exports.Also applies to: 81-81, 147-149
libs/ui-components/src/components/Repository/CreateRepository/types.ts (1)
40-51: LGTM!The
ociConfigstructure is well-designed with consistent patterns:
- Uses
OciRepoSpec.schemeandOciRepoSpec.accessModefor type safety- The
ociAuthstructure parallelshttpConfig.basicAuth- All fields are appropriately optional for form state management
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx (2)
34-37: LGTM!The privacy detection for OCI repositories correctly checks for the presence of
ociAuth, consistent with how HTTP and SSH configs are evaluated.
57-66: LGTM!The
RegistryOrUrlcomponent cleanly separates rendering logic based on repository type:
- OCI: Plain text (registry string)
- HTTP:
HttpRepositoryUrlcomponent- Git:
GitRepositoryLinkcomponentThis provides appropriate visual treatment for each repository type.
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
34-38: LGTM!Good use of the
getRepoUrlOrRegistryutility to derive the description, properly handling the abstraction between URL-based and registry-based repositories.libs/types/models/OciRepoSpec.ts (1)
1-49: Generated type looks correct for OCI repository specification.The type properly models the OCI registry configuration with appropriate optional fields for authentication, TLS settings, and access modes. The namespace enums provide type-safe values for scheme and accessMode.
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (5)
55-88: LGTM!Clean OCI-specific advanced configuration section. The structure mirrors the existing HTTP pattern with appropriate fields for basic authentication, server verification, and CA certificate.
187-214: Good refactor of repository type switching logic.The
doChangeRepoTypefunction now properly handles all three repository types (GIT, HTTP, OCI) with appropriate field initialization. OCI correctly disables resource syncs and initializes default configuration values.
250-260: LGTM!OCI radio button follows the same pattern as existing Git and HTTP options.
311-327: LGTM!Clean conditional rendering between OCI registry hostname and URL-based repository input with appropriate helper text examples.
330-373: LGTM!OCI scheme and access mode controls are well-structured with sensible default selections (HTTPS, Read-only).
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (8)
35-42: LGTM!Clean type guard and utility function for OCI-aware URL/registry resolution. These helpers properly encapsulate the abstraction between URL-based and registry-based repositories.
83-91: LGTM!Proper OCI initialization with sensible defaults (HTTPS scheme, READ access mode).
120-141: LGTM!Comprehensive loading of existing OCI repository configuration. The base64 decoding of CA certificate (line 139) correctly mirrors the encoding in
getRepository.
173-184: Good approach for repository type changes.Replacing the entire spec when the repository type changes is cleaner and safer than trying to patch individual fields, as it avoids leaving stale fields from the previous type.
188-269: LGTM!Comprehensive OCI patch handling that properly manages all OCI-specific fields including auth, CA certificate, scheme, and access mode. The logic correctly handles the
useAdvancedConfigtoggle to clean up fields when advanced config is disabled.
622-641: Static analysis false positive - Yup's conditional validation API requiresthenproperty.The Biome
noThenPropertyrule is designed to prevent accidental Promise thenable behavior, but this is standard Yupwhen()API usage for conditional validation. Thethenandotherwiseproperties are expected by Yup.
644-677: LGTM!OCI repository creation correctly builds the spec with conditional fields for authentication, TLS, and server verification settings.
570-572: Registry hostname regex correctly validates format; note Docker/OCI client behavior for single-label names.The regex correctly validates DNS/URI hostname format including FQDNs, IPv4, IPv6 (in brackets), and optional ports. Verification confirms:
- Single-label hostnames like
localhost,a, andmy-registryare accepted (DNS/URI compliant), but Docker/OCI clients treat bare single-label names (without dots or ports) as Docker Hub repository paths, not registry hostnames—except for the special caselocalhost.- IPv6 zone IDs (e.g.,
[fe80::1%eth0]) correctly fail, which is appropriate since OCI registries do not use them.Consider documenting this distinction if the validation context needs to enforce that only FQDNs or single-label names with ports are accepted as custom registry hostnames.
celdrake
force-pushed
the
EDM-2913-support-oci-repositories
branch
from
58bd472 to
055a559
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
212-219: Logic error: HttpConfigForm renders for non-HTTP repository types.The condition
repoSpec &&(line 212) will renderHttpConfigFormwheneverrepoSpecexists, regardless of repository type. Since the type assertion on line 181 doesn't perform runtime validation, Git and OCI repositories will incorrectly show the HTTP configuration form.🔎 Proposed fix
- {repoSpec && ( + {repoType === RepoSpecType.HTTP && repoSpec && ( <HttpConfigForm template={ct as HttpConfigTemplate} index={index} baseURL={repoSpec.url || ''} isReadOnly={isReadOnly} /> )}Based on past review comments, this issue was previously identified and needs to be addressed.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
libs/i18n/locales/en/translation.jsonlibs/types/index.tslibs/types/models/ApplicationVolume.tslibs/types/models/ApplicationVolumeReclaimPolicy.tslibs/types/models/DockerAuth.tslibs/types/models/K8sProviderSpec.tslibs/types/models/OciAuth.tslibs/types/models/OciAuthType.tslibs/types/models/OciRepoSpec.tslibs/types/models/OpenShiftProviderSpec.tslibs/types/models/RepoSpecType.tslibs/types/models/RepositorySpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsxlibs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsxlibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsxlibs/ui-components/src/components/Repository/CreateRepository/types.tslibs/ui-components/src/components/Repository/CreateRepository/utils.tslibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsxlibs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/form/FormSelect.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- libs/ui-components/src/components/Repository/RepositoryList.tsx
- libs/types/models/ApplicationVolume.ts
- libs/types/models/OciAuth.ts
- libs/types/models/OciAuthType.ts
- libs/types/index.ts
- libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryGeneralDetailsCard.tsx
- libs/types/models/RepositorySpec.ts
- libs/types/models/ApplicationVolumeReclaimPolicy.ts
- libs/types/models/OpenShiftProviderSpec.ts
- libs/ui-components/src/components/Repository/CreateRepository/types.ts
- libs/types/models/K8sProviderSpec.ts
- libs/ui-components/src/components/form/FormSelect.tsx
- libs/ui-components/src/components/Fleet/ImportFleetWizard/steps/RepositoryStep.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-10-10T11:35:04.458Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-10-10T11:35:04.458Z
Learning: In the flightctl-ui repository, TypeScript types are generated using hey-api/openapi-ts and output to a single index.ts file (previously used openapi-typescript-codegen with one file per type under /libs/types/models).
Applied to files:
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx
📚 Learning: 2025-11-20T09:24:41.991Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 378
File: libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts:288-326
Timestamp: 2025-11-20T09:24:41.991Z
Learning: In the flightctl-ui repository, the CreateAuthProvider form and its utilities (including getAuthProvider in libs/ui-components/src/components/AuthProvider/CreateAuthProvider/utils.ts) are specifically designed for CRUD operations on OIDC and OAuth2 authentication providers only. Other provider types (K8s, AAP, OpenShift) are not supported by this form and should be rejected with an error if loading is attempted for editing.
Applied to files:
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx
📚 Learning: 2025-02-04T09:04:36.106Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 207
File: libs/ui-components/src/components/Device/EditDeviceWizard/steps/ReviewDeviceStep.tsx:55-55
Timestamp: 2025-02-04T09:04:36.106Z
Learning: The codebase has automated CI checks ("lint" step) that verify translation coverage for all text strings, ensuring that new or modified strings have corresponding entries in translation files.
Applied to files:
libs/i18n/locales/en/translation.json
🧬 Code graph analysis (6)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx (1)
libs/types/index.ts (1)
RepoSpecType(165-165)
libs/types/models/OciRepoSpec.ts (2)
libs/types/index.ts (3)
OciRepoSpec(149-149)RepoSpecType(165-165)OciAuth(147-147)libs/types/models/OciAuth.ts (1)
OciAuth(9-9)
libs/types/models/DockerAuth.ts (1)
libs/types/index.ts (1)
DockerAuth(81-81)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (3)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)libs/types/index.ts (2)
GenericRepoSpec(113-113)HttpRepoSpec(123-123)libs/ui-components/src/types/deviceSpec.ts (1)
HttpConfigTemplate(208-214)
libs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (5)
libs/types/index.ts (3)
RepoSpecType(165-165)OciRepoSpec(149-149)Repository(161-161)libs/types/models/OciRepoSpec.ts (1)
OciRepoSpec(10-33)libs/ui-components/src/components/Repository/CreateRepository/types.ts (1)
RepositoryFormValues(10-55)libs/ui-components/src/hooks/useAppLinks.ts (1)
DEMO_REPOSITORY_URL(4-4)libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepositoryPatches(173-501)
libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (1)
getRepoUrlOrRegistry(37-42)
🪛 Biome (2.1.2)
libs/ui-components/src/components/Repository/CreateRepository/utils.ts
[error] 626-626: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Build ocp plugin
- GitHub Check: Lint
- GitHub Check: Build
🔇 Additional comments (20)
libs/i18n/locales/en/translation.json (1)
931-935: LGTM! Well-structured OCI registry translations.The new translation keys for OCI registry support are clear, consistent with existing patterns, and include helpful examples in validation messages. The authentication fields, registry configuration options, and validation messages follow the established conventions in the codebase.
Based on learnings, the automated CI checks will verify that these translation keys are properly referenced in the code.
Also applies to: 950-950, 954-955, 958-962, 980-985, 1001-1001, 1004-1004
libs/types/models/RepoSpecType.ts (1)
11-11: LGTM! OCI repository type added.The addition of the OCI enum member is straightforward and consistent with the existing repository types.
libs/ui-components/src/components/Repository/RepositoryDetails/RepositorySourceList.tsx (1)
10-10: LGTM! Correctly handles OCI registry URLs.The switch to
getRepoUrlOrRegistryproperly handles both OCI registries (which use theregistryfield) and traditional Git/HTTP repositories (which use theurlfield).Also applies to: 49-49
libs/ui-components/src/components/Repository/RepositoryDetails/RepositoryDetails.tsx (1)
96-96: LGTM! Resource syncs correctly restricted to Git repositories.The change from
!== RepoSpecType.HTTPto=== RepoSpecType.GITis more explicit and correct, especially with the addition of OCI repository support. Resource syncs are a Git-specific feature and should only display for Git repositories.libs/types/models/DockerAuth.ts (1)
1-19: LGTM! Docker authentication type well-defined.The
DockerAuthtype structure is appropriate for Docker-style OCI registry authentication with required username and password fields.libs/types/models/OciRepoSpec.ts (1)
1-50: LGTM! OCI repository specification is comprehensive.The
OciRepoSpectype provides a complete model for OCI registry configuration with:
- Clear documentation for the registry field (supports FQDN, IP addresses with optional ports)
- Appropriate enums for scheme and access mode
- Optional authentication, CA certificate, and server verification fields
The structure aligns well with OCI registry requirements.
libs/ui-components/src/components/Repository/CreateRepository/utils.ts (8)
35-42: LGTM! OCI type guard and URL helper correctly implemented.The
isOciRepoSpectype guard properly checks the repo type, andgetRepoUrlOrRegistrycleanly handles the distinction between OCI registries (usingregistryfield) and traditional repositories (usingurlfield).
83-91: LGTM! Sensible OCI defaults.Initializing OCI config with HTTPS scheme and Read-only access mode are secure and sensible defaults.
174-184: LGTM! Full spec replacement when repository type changes.Replacing the entire
/specwhenrepoTypechanges is the correct approach, as the spec structures differ significantly between Git, HTTP, and OCI repositories. This avoids leaving orphaned fields from the previous type.
189-269: LGTM! Comprehensive OCI patch handling.The OCI-specific patching logic properly handles:
- Registry field updates
- Optional advanced config fields (scheme, accessMode, skipServerVerification, CA cert)
- Conditional removal of advanced fields when
useAdvancedConfigis false- OCI authentication with proper type construction
The logic correctly removes
ca.crtwhenskipServerVerificationis enabled.
598-620: LGTM! OCI validation schema is comprehensive.The validation schema properly validates:
- Registry hostname format with helpful error message
- Scheme and accessMode enum values
- Conditional username/password requirements when auth is enabled
- CA certificate and server verification options
626-626: False positive: Yup'sthenis part of its conditional validation API.The static analysis tool is flagging the use of
thenin the Yup schema, but this is Yup's legitimate API for conditional validation with.when(). Thethenproperty is part of Yup's conditional validation configuration object, not adding athenmethod to promises.This is a known pattern in Yup and is not an issue.
645-677: LGTM! OCI repository construction handles all fields correctly.The
getRepositoryfunction for OCI repos properly:
- Sets required fields (registry, type, scheme, accessMode)
- Applies defaults (HTTPS scheme, READ access mode)
- Conditionally includes optional fields (skipServerVerification, ca.crt, ociAuth)
- Base64-encodes the CA certificate
- Constructs Docker auth with correct authType
The logic correctly excludes
ca.crtwhenskipServerVerificationis true.
571-572: Verify registry hostname regex covers all valid OCI registry formats.The regex pattern covers FQDN, IPv4, and IPv6 addresses with optional ports. Consider verifying this aligns with the backend validation pattern and covers edge cases like:
- Localhost/loopback addresses
- Non-standard ports
- International domain names (if supported)
Run the following script to check for any existing registry validation tests or patterns:
#!/bin/bash # Search for registry validation tests or patterns in the codebase rg -n -C3 'registry.*validation|registry.*regex|registry.*hostname' --type=ts --type=golibs/ui-components/src/components/Repository/CreateRepository/CreateRepositoryForm.tsx (5)
55-88: LGTM! OCI advanced configuration section well-structured.The OCI-specific advanced section properly handles:
- Basic authentication with username/password
- Server verification toggle
- CA certificate input (disabled when skipServerVerification is true)
The structure mirrors the HTTP advanced section, providing consistency in the UI.
179-214: LGTM! Repository type change handling improved with stronger typing.The refactor from
booleantoRepoSpecType | undefinedforshowConfirmChangeTypeprovides better type safety. ThedoChangeRepoTypefunction properly handles all three repository types:
- Git: enables resource syncs
- HTTP: disables resource syncs, sets config type to 'http'
- OCI: disables resource syncs, initializes OCI config with secure defaults
The initialization of
ociConfigwhen switching to OCI ensures the form state is properly set up.
250-260: LGTM! OCI radio option integrated consistently.The OCI radio button is added alongside Git and HTTP options with consistent structure and behavior.
311-373: LGTM! OCI-specific form fields properly gated.The conditional rendering correctly shows:
- Registry hostname field (with helpful examples) for OCI repositories
- Repository URL field for Git/HTTP repositories
- OCI scheme selection (HTTP/HTTPS)
- OCI access mode selection (Read only / Read and write)
All OCI fields are properly gated behind the
isOciRepocheck.
467-470: LGTM! Variable rename improves clarity.Renaming to
specPatchesmakes the variable's purpose more explicit.libs/ui-components/src/components/Device/EditDeviceWizard/steps/ConfigWithRepositoryTemplateForm.tsx (1)
181-181: Type assertion doesn't provide runtime safety.Line 181 uses a type assertion (
as GenericRepoSpec | HttpRepoSpec | undefined) which doesn't perform any runtime validation. This meansrepoSpecwill be truthy for any repository type at runtime, not just HTTP repositories.⛔ Skipped due to learnings
Learnt from: celdrake Repo: flightctl/flightctl-ui PR: 378 File: libs/ui-components/src/utils/authProvider.ts:5-18 Timestamp: 2025-11-20T07:33:34.656Z Learning: In the flightctl-ui repository, for AuthProvider objects, the metadata.name field is always present at runtime as it serves as the ID of the authprovider. The OpenAPI schema incorrectly defines it as optional (name?: string in ObjectMeta), but the type assertion `as string` is appropriate when accessing provider.metadata.name since it's guaranteed to exist.
celdrake
force-pushed
the
EDM-2913-support-oci-repositories
branch
from
055a559 to
ed48940
Compare
1 participant
Summary by CodeRabbit
New Features
Improvements
Behavior Changes
UI
✏️ Tip: You can customize this high-level summary in your review settings.