feat(exporter): add Cloud Foundry App resource export support#232
feat(exporter): add Cloud Foundry App resource export support#232gergely-szabo-sap merged 14 commits intomainfrom
Conversation
ikhandamirov
left a comment
There was a problem hiding this comment.
Looks good. Just a few remarks. Please feel free to overrule.
| }, | ||
| }, | ||
| ForProvider: v1alpha1.AppParameters{ | ||
| Name: app.GetName(), |
There was a problem hiding this comment.
Is this correct to sanitize the name? Or should it be the original one from CF?
There was a problem hiding this comment.
The CF name may not conform to the Kubernetes naming requirements. That's why it needs to be sanitized, IMO.
There was a problem hiding this comment.
But it is not a name of a Kubernetes resource, is it? Isn't it just a forProvider field unrelated to k8s?
There was a problem hiding this comment.
you're right, it is being fixed
| appWithComment.AddComment(err.Error()) | ||
| } | ||
| } | ||
| managedApp.Spec.ForProvider.NoRoute = appManifest.NoRoute |
There was a problem hiding this comment.
appManifest could be nil, as getAppManifest returns nil, nil in some cases.
There was a problem hiding this comment.
You're right, this needs to be treated.
| "context" | ||
| "log/slog" | ||
|
|
||
| // kyaml "sigs.k8s.io/yaml" |
There was a problem hiding this comment.
Minor: is this comment needed / intentional? Are you sure you don't want to use this package?
There was a problem hiding this comment.
Thank you, it's a leftover comment.
| // type docker struct { | ||
| // Image string `json:"image"` | ||
| // Username *string `json:"username,omitempty"` | ||
| // } | ||
|
|
||
| // type application struct { | ||
| // Name string `json:"name"` | ||
| // Docker *docker `json:"docker,omitempty"` | ||
| // } | ||
|
|
||
| // type manifest struct { | ||
| // Applications []application `json:"applications"` | ||
| // } |
There was a problem hiding this comment.
Minor: is this comment needed?
There was a problem hiding this comment.
Thank you, it's a leftover comment.
| Command: &process.Command, | ||
| DiskQuota: &process.DiskQuota, | ||
| Instances: process.Instances, | ||
| Memory: &process.Memory, | ||
| Timeout: &process.Timeout, | ||
| HealthCheckConfiguration: v1alpha1.HealthCheckConfiguration{ | ||
| HealthCheckType: (*string)(&process.HealthCheckType), | ||
| HealthCheckHTTPEndpoint: &process.HealthCheckHTTPEndpoint, | ||
| HealthCheckInterval: &process.HealthCheckInterval, | ||
| HealthCheckInvocationTimeout: &process.HealthCheckInvocationTimeout, |
There was a problem hiding this comment.
Command: &process.Command,
DiskQuota: &process.DiskQuota,
Memory: &process.Memory,
Timeout: &process.Timeout,
HealthCheckHTTPEndpoint: &process.HealthCheckHTTPEndpoint,
HealthCheckInterval: &process.HealthCheckInterval,
HealthCheckInvocationTimeout: &process.HealthCheckInvocationTimeout,They will always be non-nil in the output even when the CF API returned no value (empty string, zero). This differs from the CRD field definitions which are Optional and pointer-typed precisely so they can be omitted. Downstream, the controller will see these as explicitly set and may send unnecessary update calls.
| ReadinessHealthCheckType: &appManifest.ReadinessHealthCheckType, | ||
| ReadinessHealthCheckHTTPEndpoint: &appManifest.ReadinessHealthCheckHttpEndpoint, | ||
| ReadinessHealthCheckInterval: &appManifest.ReadinessHealthCheckInterval, | ||
| ReadinessHealthCheckInvocationTimeout: &appManifest.ReadinessHealthInvocationTimeout, |
There was a problem hiding this comment.
Same as above. They will always be non-nil in the output even when the CF API returned no value (empty string, zero). This differs from the CRD field definitions which are Optional and pointer-typed precisely so they can be omitted. Downstream, the controller will see these as explicitly set and may send unnecessary update calls.
| t.Errorf("expected comment to be added to secret") | ||
| } | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Minor: would it make sense to test edge cases like empty secret or user name?
68c3812 to
db4fc7e
Compare
db4fc7e to
84c52f4
Compare
a8155ac to
64de474
Compare
… documentation Add convertProcessConfiguration function to handle individual process conversion from CF manifest to Crossplane ProcessConfiguration. This extracts the process conversion logic into a separate, testable function. Changes: - convert.go: Add convertProcessConfiguration() helper, refactor convertProcessesField() to use the new helper, add documentation comments to convertProcessConfiguration and convertReadinessHealthCheckConfiguration - convert_test.go: Add TestConvertProcessConfiguration with test cases: * process_with_all_fields_set * process_with_only_required_fields * process_with_partial_fields * Also includes TestConvertReadinessHealthCheckConfiguration tests - manifest.go: Clean up commented code The new helper conditionally sets fields only when they have non-zero values, using nil pointers for omitted fields.
Add test cases covering empty username, empty secret name, and both empty to improve coverage of the docker credential secret generation.
Update version and vendorHash in config.nix.
Replace `app.GetName()` with `app.Name` in `convertAppResource`.
Modified-by: gergely-szabo-sap <gergely.szabo@sap.com>
69192b5 to
a68cc3f
Compare
Summary
This PR adds support for exporting Cloud Foundry applications to Crossplane App resources. It enables users to migrate
existing CF apps to Crossplane-managed infrastructure.
Changes
feat(exporter): adding App resource (
8e47b3e)cmd/exporter/cf/app/package with full App export functionalityapp.go: Core export logic with org/space filtering and name pattern matchingconvert.go: CF App manifest to Crossplane App CR conversionmanifest.go: CF API manifest fetching and parsingsecret.go: Docker credential secret generationcmd/exporter/main.gotest(exporter): add unit tests for convertAppResource helpers (
32e611b)TestConvertDockerField: Docker config conversion with/without credentialsTestConvertProcessesField: Process configuration conversion (nil, empty, single, multiple)TestGenerateDockerCredentialSecret: Secret generation validationdocs(exporter): add documentation comments to app export package (
6a413f1)chore(clifford): bumping xp-clifford dependency (
80ee652)github.com/SAP/xp-cliffordto latest versionTechnical Details
The App exporter:
resolveReferencesis enabledTesting
make reviewable✅