fetch k8s metadata from ocm - #45
anton-paulovich merged 4 commits into
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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 |
af67393 to
1bd1635
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Kubernetes version sync flow to source classification/metadata from the OCI/OCM component descriptor (via an embedded kubernetes-version-data localBlob) instead of fetching classifications from GitHub, and aligns the ManagedCloudProfile API/CRD accordingly.
Changes:
- Switch landscape Kubernetes version classification fetching from GitHub to an OCI localBlob referenced in the OCM component descriptor.
- Update the ManagedCloudProfile API/CRD schema (including renaming the spec field to
kubernetesUpdate) to remove GitHub source configuration and require a provider selector. - Refactor and extend unit/integration tests to validate blob-based fetching and end-to-end behavior.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Adds opencontainers/go-digest as a direct dependency for blob digest handling. |
| crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml | Renames/reshapes Kubernetes update config in the CRD and removes GitHub-related schema. |
| controllers/cloud_profile.go | Wires the controller to the renamed KubernetesUpdate config and new landscape source constructor. |
| cloudprofilesync/k8ssync/source/landscape/landscape_source.go | Implements fetching Kubernetes version classification YAML from an OCM localBlob in OCI. |
| cloudprofilesync/k8ssync/source/landscape/landscape_source_test.go | Updates tests to cover localBlob-based fetching and end-to-end OCI-only flow. |
| api/v1alpha1/zz_generated.deepcopy.go | Regenerates deepcopy code reflecting removed GitHub-related API types and renamed fields. |
| api/v1alpha1/managedcloudprofile.go | Renames the API field to kubernetesUpdate and removes GitHub source types/config. |
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Anton Paulovich <a.paulovich@sap.com> Signed-off-by: Aliaksei Dziauho <a.dziauho@sap.com>
Signed-off-by: Aliaksei Dziauho <a.dziauho@sap.com>
4784e3d to
f1bdb54
Compare
Signed-off-by: adziauho <a.dziauho@sap.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
cloudprofilesync/k8ssync/source/landscape/landscape_source.go:216
- Validate the localBlob digest before using it. Casting localReference to digest.Digest skips validation and can produce confusing fetch errors if the component descriptor contains an invalid digest string.
rc, err := s.ociRepo.Blobs().Fetch(ctx, ocispec.Descriptor{
Digest: digest.Digest(localRef),
Size: res.Access.Size,
MediaType: res.Access.MediaType,
})
cloudprofilesync/k8ssync/source/landscape/landscape_source_test.go:166
- In pushArtifactWithVersionsBlob the digest/size are computed manually even though versionsDesc already contains the canonical Digest and Size. Using versionsDesc avoids any risk of mismatch and removes redundant hashing logic.
// Push the versions YAML blob first; compute its digest.
versionsBlobBytes := []byte(versionsYAML)
h := sha256.Sum256(versionsBlobBytes)
versionsDigest := fmt.Sprintf("sha256:%x", h)
versionsSize := len(versionsBlobBytes)
crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml:660
- The CRD description for landscapeSetup.provider ends with a trailing comma, which reads like an unfinished sentence. This is user-facing schema documentation; consider making it a complete sentence.
provider:
description: Provider is the provider name to select from
the kubernetes-version-data blob,
type: string
api/v1alpha1/managedcloudprofile.go:132
- The inline API docs for LandscapeSetup.Provider end with a trailing comma. Consider making this a complete sentence since it appears in generated docs/CRD descriptions.
// Provider is the provider name to select from the kubernetes-version-data blob,
Provider string `json:"provider"`
api/v1alpha1/managedcloudprofile.go:25
- Renaming the CRD field from spec.kubernetesVersionUpdateConfig to spec.kubernetesUpdate is a breaking API change for existing ManagedCloudProfile objects. If backward compatibility is required, consider supporting both fields for a deprecation window (or providing a conversion/defaulting strategy) so older CRs keep reconciling.
// KubernetesUpdate contains the source and provider information to automate Kubernetes version updates.
// +optional
KubernetesUpdate *KubernetesVersionUpdateConfig `json:"kubernetesUpdate,omitempty"`
Signed-off-by: Aliaksei Dziauho <a.dziauho@sap.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
cloudprofilesync/k8ssync/source/landscape/landscape_source.go:216
localReferenceis used as an OCI digest via a plain type conversion (digest.Digest(localRef)) without validation. If the component descriptor contains a non-digest value, the error will surface later from the registry client and be harder to diagnose. Parse and validate the digest explicitly and fail with a clear message.
localRef := res.Access.LocalReference
if localRef == "" {
return nil, fmt.Errorf("resource %q has empty localReference", kubernetesVersionDataResourceName)
}
rc, err := s.ociRepo.Blobs().Fetch(ctx, ocispec.Descriptor{
crd/cloudprofilesync.cobaltcore.dev_managedcloudprofiles.yaml:659
- The CRD schema description for
providerends with a trailing comma and is split in a way that reads like an incomplete sentence. This is user-facing API documentation inkubectl explainand should be a complete sentence.
description: Provider is the provider name to select from
the kubernetes-version-data blob,
api/v1alpha1/managedcloudprofile.go:25
- Renaming the spec field from
kubernetesVersionUpdateConfigtokubernetesUpdateis a breaking API change for existing ManagedCloudProfile objects. I couldn't find any backwards-compat handling (no remaining references to the old JSON field name), so upgrades would silently stop applying the config until users migrate their manifests.
// KubernetesUpdate contains the source and provider information to automate Kubernetes version updates.
// +optional
KubernetesUpdate *KubernetesVersionUpdateConfig `json:"kubernetesUpdate,omitempty"`
No description provided.