Skip to content

feat(file): implement file domain - #531

Open
lukas016 wants to merge 15 commits into
feat-lk-sftpfrom
feat-lk-file-domain
Open

feat(file): implement file domain#531
lukas016 wants to merge 15 commits into
feat-lk-sftpfrom
feat-lk-file-domain

Conversation

@lukas016

@lukas016 lukas016 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This PR introduces the new file domain to controlplane by adding the v1 API types and generated CRD manifests for FileType, FileExposure, FileSubscription, and ZoneServiceConfig, along with the required kubebuilder registration, typed references, label utilities, and validation/webhook scaffolding needed to manage file-based resources and zone-scoped configuration in the platform. It also includes test coverage for handler and webhook behavior to confirm the expected lifecycle and validation logic.

@lukas016
lukas016 force-pushed the feat-lk-file-domain branch from e80b714 to 616d3b7 Compare August 3, 2026 10:20
@lukas016
lukas016 requested a review from ron96g August 3, 2026 10:22
Comment thread file/api/v1/shared_types.go
Comment thread file/api/go.sum.license Outdated
Comment thread file/config/default/kustomization.yaml
Comment thread file/internal/handler/filesubscription/handler.go
Comment thread file/internal/handler/util/getters.go Outdated
Comment thread file/internal/handler/util/publickeys.go
Comment thread file/internal/handler/zoneserviceconfig/handler.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new file domain/operator module to the controlplane monorepo, including CRDs (FileType, FileExposure, FileSubscription, ZoneServiceConfig), controllers/handlers, a validating webhook, deployment manifests, docs, and CI/release wiring.

Changes:

  • Added a new file operator module with controllers, handlers, indices, webhook validation, and samples.
  • Added CRDs + kustomize manifests (RBAC/manager/webhook/default overlays) for deploying the operator.
  • Integrated file-operator into CI and GoReleaser.

Reviewed changes

Copilot reviewed 76 out of 79 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sftp/Makefile Builds sftp manager with CGO_ENABLED=0.
file/PROJECT Kubebuilder project definition for file domain resources.
file/Makefile Build/test/lint/manifests targets for the file module.
file/internal/webhook/v1/zoneserviceconfig_webhook.go Validating webhook for ZoneServiceConfig.
file/internal/webhook/v1/zoneserviceconfig_webhook_test.go Unit tests for ZoneServiceConfig validator.
file/internal/webhook/v1/webhook_suite_test.go Envtest webhook suite bootstrap.
file/internal/index/index.go Field index registrations for file resources.
file/internal/handler/zoneserviceconfig/handler.go Reconcile logic projecting ZoneServiceConfig into identity/gateway/sftp resources.
file/internal/handler/zoneserviceconfig/handler_suite_test.go Ginkgo suite for ZoneServiceConfig handler.
file/internal/handler/util/util_suite_test.go Ginkgo suite for handler util package.
file/internal/handler/util/refs.go Helper ObjectRef builders for SFTP resources and source refs.
file/internal/handler/util/refs_test.go Tests for ref helpers and labeling helpers.
file/internal/handler/util/publickeys.go SSH key canonicalization + SFTP user/instance sync helpers.
file/internal/handler/util/publickeys_test.go Tests for public key canonicalization and sync/delete helpers.
file/internal/handler/util/labels.go Child resource labeling + selectors.
file/internal/handler/util/getters.go Getters for FileType, ZoneServiceConfig, active exposures, etc.
file/internal/handler/suite_test.go Root handler test suite.
file/internal/handler/filetype/handler.go FileType handler (selects active exposure, sets status refs/conditions).
file/internal/handler/filetype/handler_test.go Unit tests for FileType handler behavior.
file/internal/handler/filetype/handler_suite_test.go Ginkgo suite for FileType handler.
file/internal/handler/filesubscription/handler.go FileSubscription handler (approval flow + subscriber SFTP user sync).
file/internal/handler/filesubscription/handler_suite_test.go Ginkgo suite for FileSubscription handler.
file/internal/handler/fileexposure/handler.go FileExposure handler (enforces single active exposure, instance/user provisioning).
file/internal/handler/fileexposure/handler_test.go Unit tests for FileExposure handler behavior.
file/internal/handler/fileexposure/handler_suite_test.go Ginkgo suite for FileExposure handler.
file/internal/controller/zoneserviceconfig_controller.go ZoneServiceConfig controller wiring + watches/owns.
file/internal/controller/zoneserviceconfig_controller_test.go Envtest controller test for ZoneServiceConfig reconciler.
file/internal/controller/suite_test.go Envtest controller suite bootstrap for file controllers.
file/internal/controller/schema.go Scheme registration for dependent APIs.
file/internal/controller/filetype_controller.go FileType controller wiring + watch exposure events.
file/internal/controller/filetype_controller_test.go Envtest controller test for FileType reconciler.
file/internal/controller/filesubscription_controller.go FileSubscription controller wiring + watch filetype events.
file/internal/controller/filesubscription_controller_test.go Envtest controller test for FileSubscription reconciler.
file/internal/controller/fileexposure_controller.go FileExposure controller wiring + watch filetype/zoneserviceconfig events.
file/internal/controller/fileexposure_controller_test.go Envtest controller test for FileExposure reconciler.
file/go.sum.license REUSE/SPDX metadata for module go.sum.
file/go.mod File operator module dependencies and local replaces.
file/config/webhook/manifests.yaml ValidatingWebhookConfiguration manifest.
file/config/samples/file_v1_zoneserviceconfig.yaml Sample ZoneServiceConfig instance.
file/config/samples/file_v1_filetype.yaml Sample FileType instance.
file/config/samples/file_v1_filesubscription.yaml Sample FileSubscription instance.
file/config/samples/file_v1_fileexposure.yaml Sample FileExposure instance.
file/config/rbac/service_account.yaml ServiceAccount for file operator.
file/config/rbac/role.yaml ClusterRole for operator permissions.
file/config/rbac/role_binding.yaml ClusterRoleBinding for operator.
file/config/rbac/metrics_reader_role.yaml Metrics reader ClusterRole.
file/config/rbac/metrics_auth_role.yaml Metrics auth ClusterRole.
file/config/rbac/metrics_auth_role_binding.yaml Metrics auth ClusterRoleBinding.
file/config/rbac/leader_election_role.yaml Leader election Role.
file/config/rbac/leader_election_role_binding.yaml Leader election RoleBinding.
file/config/rbac/kustomization.yaml RBAC kustomize aggregation.
file/config/manager/manager.yaml Controller manager Deployment.
file/config/manager/kustomization.yaml Manager kustomization (image override).
file/config/default/namespace_patch.yaml Namespace patch enabling secret-manager integration label.
file/config/default/kustomization.yaml Default overlay kustomization.
file/config/default/deployment_patch.yaml Deployment patch adding projected secret-manager token + trust bundle mount.
file/config/crd/kustomization.yaml CRD kustomization.
file/config/crd/bases/file.cp.ei.telekom.de_zoneserviceconfigs.yaml ZoneServiceConfig CRD.
file/config/crd/bases/file.cp.ei.telekom.de_filetypes.yaml FileType CRD.
file/config/crd/bases/file.cp.ei.telekom.de_filesubscriptions.yaml FileSubscription CRD.
file/config/crd/bases/file.cp.ei.telekom.de_fileexposures.yaml FileExposure CRD.
file/cmd/file-operator/main.go Operator entrypoint (controllers, indices, metrics, webhook server).
file/api/v1/zz_generated.deepcopy.go Generated deepcopy implementations for file APIs.
file/api/v1/zoneserviceconfig_types.go ZoneServiceConfig API type definitions.
file/api/v1/shared_types.go Shared enums/types (visibility, approval strategy, SSH keys, labels).
file/api/v1/groupversion_info.go API group/version registration.
file/api/v1/filetype_types.go FileType API type definitions.
file/api/v1/filesubscription_types.go FileSubscription API type definitions.
file/api/v1/fileexposure_types.go FileExposure API type definitions.
file/api/go.sum.license REUSE/SPDX metadata for api module go.sum.
file/api/go.mod API submodule dependencies and local replaces.
file/.gitignore Ignores binaries and test artifacts for file module.
docs/docs/architecture/file.mdx Architecture documentation for the new file domain.
.goreleaser.yaml Adds file-operator build to releases.
.github/workflows/ci.yaml Adds reusable CI job for the file module.
Files not reviewed (1)
  • file/api/v1/zz_generated.deepcopy.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread file/internal/webhook/v1/zoneserviceconfig_webhook.go Outdated
Comment thread file/internal/controller/zoneserviceconfig_controller.go
Comment thread file/config/rbac/role.yaml Outdated

@ron96g ron96g left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This review was done by hand ;) I might ask AI to review it also, this might result in more comments:

  • Please check the conditions in common and align where applicable
  • Think about the caching improvement in main.go
  • I think the idea with the Active-Exposure via FileType-Status is fine but it does break our current pattern


list := &filev1.FileExposureList{}
err := r.List(ctx, list,
client.InNamespace(fileType.Namespace),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The FileType can be in a different NS. e.g. specified by team-a and exposed by team-b

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we have open discussion about it somewhere, if we want to support it. Because you can still data with this approach from another team.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do it as before

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed


list := &filev1.FileExposureList{}
if err := r.List(ctx, list,
client.InNamespace(zoneServiceConfig.Labels[cconfig.EnvironmentLabelKey]),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The FileExposure is in the team-NS or not? So it cannot be selected via environment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

Comment thread file/api/v1/fileexposure_types.go Outdated
type FileExposureSpec struct {
// Provider optionally identifies the providing application.
// +kubebuilder:validation:Optional
Provider string `json:"provider,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why optionally? And why string in favour of ObjectRef?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we will implement it later as additional field.

Provider string `json:"provider,omitempty"`

// +kubebuilder:validation:Required
FileType string `json:"fileType"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would be nice to have a comment on here. Like "This is the unique identifier of the FileType that is being exposed"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

Comment thread file/api/v1/shared_types.go Outdated
FileSolutionSFTP FileSolution = "sftp"
)

func (s FileSolution) OrDefault() FileSolution {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dead code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

metricsServerOptions.KeyName = metricsCertKey
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I propose to add a cache-selector similiar to event to avoid caching Consumer, Routes, etc. that do not belong to this domains as otherwise this will drastically increase the cache-size without any benefit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

return apiClient, nil
}

weekBeforeExp := apiClient.Status.SecretExpiresAt.Add(-7 * 24 * time.Hour)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Internal realms will have the secret-rotation disabled for now. Thus, this is dead-code. I would propose to remove it and then in the future when it comes to it, we need to find a central solution

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do you need central solution for it? Technically this client is created and manage by sftp operator. SO it should control secret rotation too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

}
}

clientSecretPath := fmt.Sprintf("zones/%s/file/%s/%s/clientSecret", zone.Name, obj.Namespace, obj.Name)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is namespace and name needed? The idea is 1 ZoneConfig per Zone

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes it is. but this for me more user-friendly because you have direct mapping between zoneserviceconfig and related credential

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

without resource name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

}

// createOrUpdateSFTPAPIClient creates or updates an identity Client for the SFTP API.
func createOrUpdateSFTPAPIClient(ctx context.Context, obj *filev1.ZoneServiceConfig, zone *adminv1.Zone) (*identityv1.Client, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would propose to refactor this function and remove the creation part into a separate function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

secretsapi.WithSecretValue(clientSecretPath, secretValue),
}

availableSecret, err := secretsapi.API().UpsertEnvironment(ctx, zone.Labels[cconfig.EnvironmentLabelKey], options...)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

zone.Labels[cconfig.EnvironmentLabelKey] can be replaced by contextutil.EnvFromContextOrDie

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pushed

@lukas016
lukas016 force-pushed the feat-lk-file-domain branch from 972c03f to e51cdf3 Compare September 2, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants