Fix logic for tags creation date retrieving#15
Merged
valeryia-hurynovich merged 7 commits intomasterfrom Apr 14, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoves manifest timestamp extraction from OCI sources and the Changes
Sequence DiagramsequenceDiagram
participant Reconciler as Reconciler
participant Keppel as Keppel API
participant K8s as Kubernetes API
Reconciler->>Keppel: GET {registry}/v1/repositories/{repo}/_manifests
Note right of Keppel: returns JSON {manifests:[{tags, pushed_at,...}]}
Keppel-->>Reconciler: map[tagName] = pushed_at (unix → time.Time)
Reconciler->>K8s: Read CloudProfile (machine image versions)
K8s-->>Reconciler: CloudProfile{versions...}
Reconciler->>Reconciler: Compute GC candidates by comparing Keppel pushed_at vs referenced versions
Reconciler->>K8s: Update CloudProfile (remove unreferenced old versions)
K8s-->>Reconciler: Updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 76-80: The cleanup polls call k8sClient.List and discard the error
(`_ = k8sClient.List(...)`), which can hide transient API errors and let
Eventually succeed incorrectly; update the poll closures (the ones using
Eventually that create `var updated v1alpha1.ManagedCloudProfileList` and call
`k8sClient.List(ctx, &updated)`) to capture the error, assert or fail the poll
on non-nil error (e.g., return a sentinel non-zero count or call
Expect(err).To(BeNil()) inside the closure), and only return len(updated.Items)
when err == nil so transient List errors don’t appear as “0 items.” Ensure
similar fixes are applied to the other poll blocks referenced (around the
102–118 region).
- Around line 540-560: Don't instantiate and call a second
controllers.Reconciler (the local reconciler variable with fakeFactory and
FetchKeppelTagsFunc) against the same object; instead replace/override the
shared test reconciler used by the envtest manager with your configured
&controllers.Reconciler{...} (including Client: k8sClient, OCISourceFactory:
&fakeFactory{}, FetchKeppelTagsFunc: ...) so the manager's controller uses the
fake dependencies; remove the direct reconciler.Reconcile(...) call and ensure
you still create the MCP with k8sClient.Create(ctx, mcp) so the manager's
reconciler processes it.
In `@controllers/managedcloudprofile_controller.go`:
- Line 50: The Keppel tag-fetch hook (FetchKeppelTagsFunc) currently performs
anonymous HTTPS calls; update its usage and signature so OCI auth/TLS settings
gathered for the OCI source are passed through: include the auth credentials and
TLS/Insecure flag (or an http.RoundTripper/transport built from them) to
FetchKeppelTagsFunc and use that transport for requests. Locate the
FetchKeppelTagsFunc type and all call sites (including where OCI source settings
are parsed) and modify the invocation to supply the existing OCI auth/tls config
(or a transport derived from it), and update any implementations of
FetchKeppelTagsFunc to honor the provided auth/tls parameters so registries
requiring credentials or Insecure=true work correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb634736-a566-46e4-8d8a-d7c328ff88dd
📒 Files selected for processing (3)
cloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
| client.Client | ||
| OCISourceFactory OCISourceFactory | ||
| OCISourceFactory OCISourceFactory | ||
| FetchKeppelTagsFunc func(ctx context.Context, registry, repository string) (map[string]time.Time, error) |
There was a problem hiding this comment.
Pass OCI auth/TLS settings into the Keppel fetch path.
This hook can only make anonymous HTTPS requests right now. GC will start failing against registries that require credentials or Insecure: true, even though those settings are already read for the OCI source a few lines earlier.
Also applies to: 187-190, 483-515
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controllers/managedcloudprofile_controller.go` at line 50, The Keppel
tag-fetch hook (FetchKeppelTagsFunc) currently performs anonymous HTTPS calls;
update its usage and signature so OCI auth/TLS settings gathered for the OCI
source are passed through: include the auth credentials and TLS/Insecure flag
(or an http.RoundTripper/transport built from them) to FetchKeppelTagsFunc and
use that transport for requests. Locate the FetchKeppelTagsFunc type and all
call sites (including where OCI source settings are parsed) and modify the
invocation to supply the existing OCI auth/tls config (or a transport derived
from it), and update any implementations of FetchKeppelTagsFunc to honor the
provided auth/tls parameters so registries requiring credentials or
Insecure=true work correctly.
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
4abe499 to
957c302
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/managedcloudprofile_controller.go (1)
192-197: 🛠️ Refactor suggestion | 🟠 MajorDead code:
versionsfromsrc.GetVersionsis fetched but never used.After switching to Keppel tags for GC candidate selection, the OCI source versions are retrieved but not utilized. This creates confusion and unnecessary API calls.
Either remove this code block if OCI versions are no longer needed for GC, or clarify the intent if they serve another purpose.
♻️ Proposed fix to remove unused code
- log.V(1).Info("retrieving source versions", "image", updates.ImageName) - versions, err := src.GetVersions(ctx) - if err != nil { - return r.failWithStatusUpdate(ctx, mcp, fmt.Errorf("failed to list source versions for garbage collection: %w", err)) - } - log.V(1).Info("retrieved source versions", "count", len(versions), "image", updates.ImageName) - referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName, log)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 192 - 197, The block fetching OCI source versions (calls to src.GetVersions and the local variable versions plus the surrounding log.V(1).Info statements that reference "retrieving source versions" and "retrieved source versions") is dead code and should be removed; delete the call to src.GetVersions(ctx), the error handling and the two log.V(1).Info lines (or, if the versions are actually needed, replace the current Keppel-tag GC flow to consume the returned versions where GC candidates are computed), ensuring any references to the local variable versions are also removed or replaced so there are no unused variables remaining.
🧹 Nitpick comments (2)
controllers/managedcloudprofile_controller.go (2)
491-491: Consider reusinghttp.Clientinstead of creating one per call.Creating a new
http.Clientfor eachfetchKeppelTagscall prevents connection reuse. Consider making it a package-level variable or a field on the Reconciler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` at line 491, The fetchKeppelTags function currently creates a new http.Client each call (httpClient := &http.Client{Timeout: 30 * time.Second}); change this to reuse a single client instance by moving the client to a longer-lived scope — either add a package-level variable (e.g., var keppelHTTPClient = &http.Client{Timeout: 30 * time.Second}) or add a field on the Reconciler struct (e.g., HTTPClient *http.Client) and initialize it once, then replace the local httpClient usage in fetchKeppelTags to use that shared client to enable connection reuse.
170-185: OCI source creation may be unnecessary for garbage collection.If the GC logic now relies solely on Keppel tags (lines 206-215), the OCI source creation and credential fetching (lines 170-185) become unused. Consider whether this code block should be removed or if the OCI source serves another purpose in GC.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 170 - 185, The OCI credential fetch and OCISource creation (r.getCredential(...) and r.OCISourceFactory.Create(...)) appear unused by the garbage collection path that only relies on Keppel tags; remove the credential retrieval and OCISource initialization from the GC branch (the code referencing updates.Source.OCI, password, src) and any associated variables or error branches (including the failWithStatusUpdate calls tied solely to those operations), then run a build and search for usages of src/password to ensure nothing else depends on them; if OCISource is actually required for some GC subpath, instead add a clear comment and gate its creation behind that specific condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controllers/managedcloudprofile_controller.go`:
- Around line 507-512: The code builds tagMap from result.Manifests and uses
time.Unix(t.PushedAt, 0) which yields 1970-01-01 for t.PushedAt==0; change the
loop that iterates over result.Manifests -> m.Tags and t (t.Name, t.PushedAt) to
skip tags where t.PushedAt == 0 (or optionally log a warning via the controller
logger) instead of inserting time.Unix(0, 0) into tagMap so tags with unset
timestamps are not treated as ancient and scheduled for deletion.
- Around line 483-489: fetchKeppelTags currently calls keppelURL and proceeds to
create an HTTP request without validating keppelURL's result; update
fetchKeppelTags to check keppelURL's return (e.g., treat an empty string or an
error return as an invalid repository) and return a clear descriptive error
(including repository and registry) before attempting
http.NewRequestWithContext; reference fetchKeppelTags and keppelURL when making
this change so callers get a handled error for invalid repo formats.
---
Outside diff comments:
In `@controllers/managedcloudprofile_controller.go`:
- Around line 192-197: The block fetching OCI source versions (calls to
src.GetVersions and the local variable versions plus the surrounding
log.V(1).Info statements that reference "retrieving source versions" and
"retrieved source versions") is dead code and should be removed; delete the call
to src.GetVersions(ctx), the error handling and the two log.V(1).Info lines (or,
if the versions are actually needed, replace the current Keppel-tag GC flow to
consume the returned versions where GC candidates are computed), ensuring any
references to the local variable versions are also removed or replaced so there
are no unused variables remaining.
---
Nitpick comments:
In `@controllers/managedcloudprofile_controller.go`:
- Line 491: The fetchKeppelTags function currently creates a new http.Client
each call (httpClient := &http.Client{Timeout: 30 * time.Second}); change this
to reuse a single client instance by moving the client to a longer-lived scope —
either add a package-level variable (e.g., var keppelHTTPClient =
&http.Client{Timeout: 30 * time.Second}) or add a field on the Reconciler struct
(e.g., HTTPClient *http.Client) and initialize it once, then replace the local
httpClient usage in fetchKeppelTags to use that shared client to enable
connection reuse.
- Around line 170-185: The OCI credential fetch and OCISource creation
(r.getCredential(...) and r.OCISourceFactory.Create(...)) appear unused by the
garbage collection path that only relies on Keppel tags; remove the credential
retrieval and OCISource initialization from the GC branch (the code referencing
updates.Source.OCI, password, src) and any associated variables or error
branches (including the failWithStatusUpdate calls tied solely to those
operations), then run a build and search for usages of src/password to ensure
nothing else depends on them; if OCISource is actually required for some GC
subpath, instead add a clear comment and gate its creation behind that specific
condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34978a07-b8a9-4987-a1eb-95ec04f3c689
📒 Files selected for processing (3)
cloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
🚧 Files skipped from review as they are similar to previous changes (1)
- controllers/managedcloudprofile_controller_test.go
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
957c302 to
c3c812a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
controllers/managedcloudprofile_controller.go (2)
521-525:⚠️ Potential issue | 🟡 MinorIgnore tags with an unset
pushed_at.
time.Unix(0, 0)turns a missing timestamp into1970-01-01, which makes the tag look ancient and immediately eligible for deletion.🛡️ Suggested fix
tagMap := make(map[string]time.Time) for _, m := range result.Manifests { for _, t := range m.Tags { + if t.PushedAt == 0 { + continue + } tagMap[t.Name] = time.Unix(t.PushedAt, 0) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 521 - 525, The current loop unconditionally maps tags into tagMap using time.Unix(t.PushedAt, 0), which converts an unset pushed_at (0) into the 1970 epoch and causes stale-tag deletion; update the loop that iterates result.Manifests -> m.Tags to skip any tag where t.PushedAt == 0 (or <= 0) and only set tagMap[t.Name] = time.Unix(t.PushedAt, 0) when the timestamp is present, leaving tags with no pushed_at out of tagMap.
50-51:⚠️ Potential issue | 🔴 CriticalThread OCI auth/TLS settings into the Keppel fetcher.
reconcileGarbageCollection()already resolvesUsername,Password, andInsecure, but this hook still only acceptsregistry/repository. That meansfetchKeppelTags()always makes an anonymous, certificate-validating HTTPS request, so GC still breaks for private registries or registries that requireInsecure: true.🛠️ Minimal shape of the fix
type Reconciler struct { client.Client OCISourceFactory OCISourceFactory - FetchKeppelTagsFunc func(ctx context.Context, registry, repository string) (map[string]time.Time, error) + FetchKeppelTagsFunc func(ctx context.Context, params cloudprofilesync.OCIParams, insecure bool) (map[string]time.Time, error) } ... - tags, err := r.FetchKeppelTagsFunc(ctx, updates.Source.OCI.Registry, updates.Source.OCI.Repository) + tags, err := r.FetchKeppelTagsFunc(ctx, cloudprofilesync.OCIParams{ + Registry: updates.Source.OCI.Registry, + Repository: updates.Source.OCI.Repository, + Username: updates.Source.OCI.Username, + Password: string(password), + }, updates.Source.OCI.Insecure)Also applies to: 188-190, 484-504
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 50 - 51, reconcileGarbageCollection resolves Username, Password and Insecure but the Keppel tag fetcher signature (FetchKeppelTagsFunc / fetchKeppelTags) only accepts registry/repository which forces anonymous, TLS-validating requests; update the FetchKeppelTagsFunc signature and all callers (including reconcileGarbageCollection and places around lines referenced) to accept and forward auth/TLS parameters (username, password, insecure bool or an auth/TLS config object), then change fetchKeppelTags to use those credentials and the insecure flag when creating the HTTP/OCI client (or via OCISourceFactory) so private registries and Insecure:true are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 36-49: The fake OCI helper types fakeOCISource (and its
GetVersions method) and fakeFactory (and its Create method) are unused and
causing linter failures; either remove these types entirely from the test file
or reintroduce a test/spec that exercises them (e.g., restore or add a test that
calls fakeFactory.Create and uses the returned fakeOCISource.GetVersions) so
they become referenced; update imports if removing them to avoid unused import
errors.
In `@controllers/managedcloudprofile_controller.go`:
- Around line 521-525: The tag keys are using raw Keppel tag names (t.Name)
which need the same normalization as cloudprofilesync/source.go (replace '_'
with '+') so versions match deleteVersions() and GC; update the loop that builds
tagMap (iterating result.Manifests -> m.Tags -> t.Name) to normalize the tag
name (e.g., strings.ReplaceAll(t.Name, "_", "+") or call the existing normalize
function if present) before using it as the map key (tagMap[...] =
time.Unix(...)).
---
Duplicate comments:
In `@controllers/managedcloudprofile_controller.go`:
- Around line 521-525: The current loop unconditionally maps tags into tagMap
using time.Unix(t.PushedAt, 0), which converts an unset pushed_at (0) into the
1970 epoch and causes stale-tag deletion; update the loop that iterates
result.Manifests -> m.Tags to skip any tag where t.PushedAt == 0 (or <= 0) and
only set tagMap[t.Name] = time.Unix(t.PushedAt, 0) when the timestamp is
present, leaving tags with no pushed_at out of tagMap.
- Around line 50-51: reconcileGarbageCollection resolves Username, Password and
Insecure but the Keppel tag fetcher signature (FetchKeppelTagsFunc /
fetchKeppelTags) only accepts registry/repository which forces anonymous,
TLS-validating requests; update the FetchKeppelTagsFunc signature and all
callers (including reconcileGarbageCollection and places around lines
referenced) to accept and forward auth/TLS parameters (username, password,
insecure bool or an auth/TLS config object), then change fetchKeppelTags to use
those credentials and the insecure flag when creating the HTTP/OCI client (or
via OCISourceFactory) so private registries and Insecure:true are handled
correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da8e00c9-8d42-4d36-abc1-ce436e18488e
📒 Files selected for processing (3)
cloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
c3c812a to
0412853
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
controllers/managedcloudprofile_controller_test.go (2)
36-49:⚠️ Potential issue | 🟠 MajorRemove or re-enable these fake OCI helpers.
fakeOCISourceandfakeFactoryare still unreachable, andgolangci-lintis already failing on them as unused. Either delete them in this PR or restore an active spec that exercises them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller_test.go` around lines 36 - 49, The test contains unused helpers fakeOCISource and fakeFactory which cause golangci-lint failures; either delete these types (fakeOCISource and fakeFactory and their methods GetVersions and Create) from controllers/managedcloudprofile_controller_test.go, or re-enable them by adding/restoring a test/spec that constructs fakeFactory, calls Create, and exercises fakeOCISource.GetVersions so the helpers are referenced; update imports if necessary after removal or ensure the new test uses these symbols.
315-593:⚠️ Potential issue | 🟠 MajorPlease restore the GC specs instead of checking them in commented out.
This PR changes
reconcileGarbageCollection(),deleteVersions(), and the ProviderConfig pruning path, but the end-to-end GC scenarios are now disabled. That leaves the new Keppel flow effectively untested and is also why the fake OCI helpers above are dead code.Also applies to: 761-852
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller_test.go` around lines 315 - 593, Restore the end-to-end garbage collection tests that were commented out in managedcloudprofile_controller_test.go so the changes to reconcileGarbageCollection, deleteVersions and the ProviderConfig pruning path are validated; specifically re-enable the three commented It blocks that exercise GC behavior (the tests asserting deletion of old machine image versions, preservation when referenced by Shoot worker pools, and preservation for Shoot workers), ensure the fake OCI helpers (fakeFactory and FetchKeppelTagsFunc) are used by the reconciler in those tests, and update any test setup (ManagedCloudProfile/CloudProfile/Shoot objects and MachineImageUpdate/ GarbageCollection config) to match the new reconcile/deleteVersions/provider-config pruning behavior so tests compile and assert the expected outcomes.controllers/managedcloudprofile_controller.go (2)
523-525:⚠️ Potential issue | 🟠 MajorSkip tags with missing
pushed_at.
time.Unix(0, 0)turns an unset Keppel timestamp into1970-01-01, so those tags become immediate GC candidates and can be deleted incorrectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 523 - 525, The loop over m.Tags incorrectly treats missing pushed timestamps as time.Unix(0,0); update the loop that processes m.Tags (using t.Name, t.PushedAt, normalizedTag, and tagMap) to skip any tag where t.PushedAt is zero/unset (e.g., if t.PushedAt == 0 { continue }) so only tags with a valid pushed_at get added to tagMap with time.Unix(t.PushedAt, 0).
50-51:⚠️ Potential issue | 🔴 CriticalThread OCI auth/TLS settings through the Keppel fetch hook.
This callback still only receives
registryandrepository, so the username/password andInsecuresettings already read for the OCI source can never reach the HTTP request. GC will still fail against private registries or registries that require insecure transport.Also applies to: 188-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 50 - 51, The FetchKeppelTagsFunc currently only takes (ctx, registry, repository) so OCI auth and TLS settings aren't propagated; change the FetchKeppelTagsFunc type to accept the OCI source auth/TLS info (e.g., add parameters for username, password and insecure bool, or pass the OCISource/OCISourceFactory result) and update all places that assign or call FetchKeppelTagsFunc (including where OCISourceFactory is used and the controller methods around the existing FetchKeppelTagsFunc usage and the lines referenced 188-190) to supply those values; inside the Keppel fetch implementation use the provided username/password to set the Authorization header (or basic auth) and use the insecure flag to configure the HTTP client's TLS/transport settings so requests succeed against private or insecure registries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controllers/managedcloudprofile_controller.go`:
- Around line 533-543: The current construction of the Keppel _manifests URL
accepts repository strings with empty segments (e.g., "/repo", "account/",
"account//nested"); update the validation where you split repository into parts
to require len(parts) >= 2 and also ensure parts[0] (account) is non-empty and
every element in parts[1:] (repository path segments) is non-empty; if any are
empty return an error like "invalid repository format: %q, must be
<account>/<repo> with non-empty segments" instead of building the URL, then
proceed to format the URL using registry, parts[0], and strings.Join(parts[1:],
"/").
---
Duplicate comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 36-49: The test contains unused helpers fakeOCISource and
fakeFactory which cause golangci-lint failures; either delete these types
(fakeOCISource and fakeFactory and their methods GetVersions and Create) from
controllers/managedcloudprofile_controller_test.go, or re-enable them by
adding/restoring a test/spec that constructs fakeFactory, calls Create, and
exercises fakeOCISource.GetVersions so the helpers are referenced; update
imports if necessary after removal or ensure the new test uses these symbols.
- Around line 315-593: Restore the end-to-end garbage collection tests that were
commented out in managedcloudprofile_controller_test.go so the changes to
reconcileGarbageCollection, deleteVersions and the ProviderConfig pruning path
are validated; specifically re-enable the three commented It blocks that
exercise GC behavior (the tests asserting deletion of old machine image
versions, preservation when referenced by Shoot worker pools, and preservation
for Shoot workers), ensure the fake OCI helpers (fakeFactory and
FetchKeppelTagsFunc) are used by the reconciler in those tests, and update any
test setup (ManagedCloudProfile/CloudProfile/Shoot objects and
MachineImageUpdate/ GarbageCollection config) to match the new
reconcile/deleteVersions/provider-config pruning behavior so tests compile and
assert the expected outcomes.
In `@controllers/managedcloudprofile_controller.go`:
- Around line 523-525: The loop over m.Tags incorrectly treats missing pushed
timestamps as time.Unix(0,0); update the loop that processes m.Tags (using
t.Name, t.PushedAt, normalizedTag, and tagMap) to skip any tag where t.PushedAt
is zero/unset (e.g., if t.PushedAt == 0 { continue }) so only tags with a valid
pushed_at get added to tagMap with time.Unix(t.PushedAt, 0).
- Around line 50-51: The FetchKeppelTagsFunc currently only takes (ctx,
registry, repository) so OCI auth and TLS settings aren't propagated; change the
FetchKeppelTagsFunc type to accept the OCI source auth/TLS info (e.g., add
parameters for username, password and insecure bool, or pass the
OCISource/OCISourceFactory result) and update all places that assign or call
FetchKeppelTagsFunc (including where OCISourceFactory is used and the controller
methods around the existing FetchKeppelTagsFunc usage and the lines referenced
188-190) to supply those values; inside the Keppel fetch implementation use the
provided username/password to set the Authorization header (or basic auth) and
use the insecure flag to configure the HTTP client's TLS/transport settings so
requests succeed against private or insecure registries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b595adf-dd02-4557-bb16-17fdeb334edf
📒 Files selected for processing (3)
cloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
0412853 to
0cc52f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
controllers/managedcloudprofile_controller_test.go (2)
813-852:⚠️ Potential issue | 🟠 MajorThis spec still doesn't exercise ProviderConfig pruning.
The suite registry only seeds
account/repo, but this case points the live controller atrepo/provider-config-image. The expectedFailedReconcileStatusand unchangedProviderConfigtherefore only prove the reconcile failed beforedeleteVersions()ran.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller_test.go` around lines 813 - 852, The test currently points the live controller at repository "repo/provider-config-image" (in mcp.Spec) but the suite registry only seeds "account/repo", so the test never reaches deleteVersions() and only verifies reconcile failure; fix by making the registry seed match the repository referenced by the test or change the test repository to one already seeded: update the suite registry seeding to include "repo/provider-config-image" (or change mcp.Spec.Image.Repository to the seeded "account/repo") so that garbage collection (mcp.Spec.GarbageCollection) actually exercises ProviderConfig pruning and triggers deleteVersions().
556-575:⚠️ Potential issue | 🟠 MajorDon't race the live manager with a second reconciler here.
k8sClient.Create(ctx, mcp)already queues the manager-backed controller started incontrollers/suite_test.go. Calling anothercontrollers.Reconcilerdirectly on the same object makes the outcome timing-dependent and bypasses the shared test dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller_test.go` around lines 556 - 575, The test is starting a second reconciler directly (controllers.Reconciler and calling Reconcile) which races the manager-backed controller; remove the explicit instantiation of controllers.Reconciler and the direct call to Reconcile, and instead rely on the manager-backed controller started in suite_test.go by asserting via the Kubernetes client that the ManagedCloudProfile (mcp) was reconciled (e.g., use Eventually with k8sClient.Get or k8sClient.Status().Get to observe expected status/conditions or resource updates made by the controller) rather than checking a ReconcileResult.RequeueAfter value from a manual Reconcile invocation.controllers/managedcloudprofile_controller.go (3)
50-52:⚠️ Potential issue | 🔴 CriticalPass OCI auth and TLS settings into the Keppel fetch path.
This callback only receives
registryandrepository, so the default implementation at Line 485 has no way to honorupdates.Source.OCI.Insecure,Username, orPassword. GC will fail against HTTP or authenticated registries even though the OCI source above is configured for them.Also applies to: 188-190, 484-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 50 - 52, The FetchKeppelTagsFunc callback currently only accepts (ctx, registry, repository) and therefore cannot honor per-source OCI settings (updates.Source.OCI.Insecure, Username, Password); change its signature to accept the OCISource (or explicit auth/tls params) e.g. func(ctx context.Context, source OCISource, registry, repository string) (map[string]time.Time, error) or include (insecure bool, username, password string), update the default implementation (the function used to fetch Keppel tags) to construct the OCI client via OCISourceFactory using those settings, and update all call sites to pass the OCISource or auth/tls values so authenticated and HTTP registries work correctly.
554-558:⚠️ Potential issue | 🟡 MinorReject empty nested repo segments and clear the red linter line.
SplitN(repository, "/", 2)still accepts values likeaccount//nested, which formats/repositories//nested/_manifests. While touching this helper, combine the named return types to(account, repo string, err error)to fix the currentparamTypeCombinefailure.🔧 Suggested fix
-func splitKeppelRepository(repository string) (account string, repo string, err error) { +func splitKeppelRepository(repository string) (account, repo string, err error) { parts := strings.SplitN(repository, "/", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { return "", "", fmt.Errorf("invalid repository format %q, must be <account>/<repository-path>", repository) } + for _, segment := range strings.Split(parts[1], "/") { + if segment == "" { + return "", "", fmt.Errorf("invalid repository format %q, must be <account>/<repository-path>", repository) + } + } return parts[0], parts[1], nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 554 - 558, The helper splitKeppelRepository should both combine the named return types to (account, repo string, err error) and reject repository strings that contain empty nested segments (e.g., "account//nested"); after splitting into parts := strings.SplitN(repository, "/", 2) keep the existing checks for parts length and parts[0] non-empty, then validate the repo part by doing a secondary split like strings.Split(parts[1], "/") and ensure no segment is the empty string—return an error if any empty segment is found.
523-526:⚠️ Potential issue | 🟡 MinorIgnore tags with missing push timestamps.
time.Unix(0, 0)turns an unsetpushed_atinto 1970-01-01, so the tag will always look old enough to delete. Skip zero timestamps or fall back to the manifest timestamp before writing totagMap.🔧 Suggested fix
tagMap := make(map[string]time.Time) for _, m := range result.Manifests { for _, t := range m.Tags { + pushedAt := t.PushedAt + if pushedAt == 0 { + pushedAt = m.PushedAt + } + if pushedAt == 0 { + continue + } normalizedTag := strings.ReplaceAll(t.Name, "_", "+") - tagMap[normalizedTag] = time.Unix(t.PushedAt, 0) + tagMap[normalizedTag] = time.Unix(pushedAt, 0) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 523 - 526, The code currently adds tags with t.PushedAt==0 (which becomes 1970-01-01) into tagMap; update the loop over result.Manifests and m.Tags to skip any tag where t.PushedAt == 0 or, if you prefer a fallback, use the manifest timestamp m.PushedAt (or another manifest-level timestamp) when t.PushedAt==0 before writing to tagMap; ensure you still normalize the tag (normalizedTag) and only set tagMap[normalizedTag] when you have a non-zero time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 313-379: The test never triggers the single-version GC because
mcp.Spec.MachineImageUpdates is empty and the created reconciler is discarded;
fix by (1) populating mcp.Spec.MachineImageUpdates with an update entry for
"gc-image" so reconcileGarbageCollection() has an OCI update to process
(reference mcp.Spec.MachineImageUpdates and the machine image name "gc-image"),
(2) stop discarding the reconciler (remove "_ = reconciler" so the variable
reconciler is actually used), and (3) adjust the fake FetchKeppelTagsFunc
timestamps (or the MaxAge) so that newVersion ("1.0.0") is newer than the MaxAge
cutoff (e.g., compute times relative to time.Now() or make base very recent) so
only oldVersion ("0.1.0") is eligible for deletion.
- Around line 383-385: The local test variable amd64 in the test function
shadows the suite-level amd64 variable and triggers govet; remove the local
declaration and reuse the outer suite-level amd64 (or if you intended a
different value, rename the local to a distinct identifier), updating the
assignment near the usage of version := "1.0.0" so only version is declared
locally and amd64 refers to the existing suite variable.
---
Duplicate comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 813-852: The test currently points the live controller at
repository "repo/provider-config-image" (in mcp.Spec) but the suite registry
only seeds "account/repo", so the test never reaches deleteVersions() and only
verifies reconcile failure; fix by making the registry seed match the repository
referenced by the test or change the test repository to one already seeded:
update the suite registry seeding to include "repo/provider-config-image" (or
change mcp.Spec.Image.Repository to the seeded "account/repo") so that garbage
collection (mcp.Spec.GarbageCollection) actually exercises ProviderConfig
pruning and triggers deleteVersions().
- Around line 556-575: The test is starting a second reconciler directly
(controllers.Reconciler and calling Reconcile) which races the manager-backed
controller; remove the explicit instantiation of controllers.Reconciler and the
direct call to Reconcile, and instead rely on the manager-backed controller
started in suite_test.go by asserting via the Kubernetes client that the
ManagedCloudProfile (mcp) was reconciled (e.g., use Eventually with
k8sClient.Get or k8sClient.Status().Get to observe expected status/conditions or
resource updates made by the controller) rather than checking a
ReconcileResult.RequeueAfter value from a manual Reconcile invocation.
In `@controllers/managedcloudprofile_controller.go`:
- Around line 50-52: The FetchKeppelTagsFunc callback currently only accepts
(ctx, registry, repository) and therefore cannot honor per-source OCI settings
(updates.Source.OCI.Insecure, Username, Password); change its signature to
accept the OCISource (or explicit auth/tls params) e.g. func(ctx
context.Context, source OCISource, registry, repository string)
(map[string]time.Time, error) or include (insecure bool, username, password
string), update the default implementation (the function used to fetch Keppel
tags) to construct the OCI client via OCISourceFactory using those settings, and
update all call sites to pass the OCISource or auth/tls values so authenticated
and HTTP registries work correctly.
- Around line 554-558: The helper splitKeppelRepository should both combine the
named return types to (account, repo string, err error) and reject repository
strings that contain empty nested segments (e.g., "account//nested"); after
splitting into parts := strings.SplitN(repository, "/", 2) keep the existing
checks for parts length and parts[0] non-empty, then validate the repo part by
doing a secondary split like strings.Split(parts[1], "/") and ensure no segment
is the empty string—return an error if any empty segment is found.
- Around line 523-526: The code currently adds tags with t.PushedAt==0 (which
becomes 1970-01-01) into tagMap; update the loop over result.Manifests and
m.Tags to skip any tag where t.PushedAt == 0 or, if you prefer a fallback, use
the manifest timestamp m.PushedAt (or another manifest-level timestamp) when
t.PushedAt==0 before writing to tagMap; ensure you still normalize the tag
(normalizedTag) and only set tagMap[normalizedTag] when you have a non-zero
time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5b7ad2d-31fe-429f-990b-3d3f10a83b1a
📒 Files selected for processing (4)
cloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.gocontrollers/suite_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
0cc52f2 to
b2eedcf
Compare
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
e2ac99a to
e8a4a71
Compare
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
34e57af to
c409bb8
Compare
adziauho
suggested changes
Apr 14, 2026
adziauho
suggested changes
Apr 14, 2026
anton-paulovich
requested changes
Apr 14, 2026
Collaborator
There was a problem hiding this comment.
And can u also add one of my prev suggestions? #15 (comment)
Comment on lines
+485
to
+488
| log, err := logr.FromContext(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to extract logger from context: %w", err) | ||
| } |
Collaborator
There was a problem hiding this comment.
I can't find the code where u put it into contexrt. So now we will alsways have an error
Collaborator
Author
There was a problem hiding this comment.
ctrl.LoggerFrom(ctx) doesn’t require anything to be put into the context. If no logger is present, controller runtime returns a fallback logger, so there is no error case
Collaborator
There was a problem hiding this comment.
valeryia-hurynovich
force-pushed
the
logic-to-receive-pushed-date
branch
from
2c39a1c to
70ede21
Compare
Merging this branch changes the coverage (1 decrease, 1 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Collaborator
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
controllers/managedcloudprofile_controller.go (2)
486-524:⚠️ Potential issue | 🟠 MajorOCI auth/TLS settings not passed to Keppel fetch path.
The
fetchKeppelTagsfunction:
- Hardcodes
insecure=false(line 491), ignoringupdate.Source.OCI.Insecure- Makes anonymous requests without passing OCI credentials
GC will fail against registries requiring authentication or
Insecure: truemode, even though these settings are already configured for the OCI source.Consider passing the OCI configuration to
fetchKeppelTagsor throughRegistryClient.GetTags:type RegistryClient interface { - GetTags(ctx context.Context, registry, repository string) (map[string]time.Time, error) + GetTags(ctx context.Context, registry, repository, username, password string, insecure bool) (map[string]time.Time, error) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 486 - 524, fetchKeppelTags is creating an anonymous HTTP client and calls registryBaseURL with a hardcoded insecure=false, so OCI credentials and Insecure TLS settings from the source are ignored; update fetchKeppelTags to accept the OCI config or a prepared registry client (e.g., pass in the source's OCI Insecure flag and auth/credentials or a RegistryClient instance), use that config when calling registryBaseURL (instead of the hardcoded false), and use the same auth/TLS settings when building the request/HTTP client (or delegate the call to RegistryClient.GetTags/keppelURL builder) so authenticated and Insecure TLS registry access works correctly (refer to fetchKeppelTags, registryBaseURL, keppelURL, RegistryClient.GetTags, and the http.Client/TLSClientConfig creation in the diff).
558-569:⚠️ Potential issue | 🟠 MajorNormalize tag names when storing in tagMap to match CloudProfile version format.
Raw Keppel tag names are stored without normalization, but CloudProfile versions are normalized (underscores replaced with plus signs). This causes garbage collection to fail: when checking if a tag is referenced via
referencedVersions[tag], tags like1.0.1_abcdon't match the CloudProfile entries stored as1.0.1+abc, so the lookup fails and versions are never found as referenced.🔧 Proposed fix
for _, t := range m.Tags { if t.PushedAt == 0 { log.V(1).Info("tag without pushed at", "name", t.Name) continue } log.V(2).Info("processing tag", "tag", t.Name, "pushedAt", t.PushedAt, ) - tagMap[t.Name] = time.Unix(t.PushedAt, 0) + name := strings.ReplaceAll(t.Name, "_", "+") + tagMap[name] = time.Unix(t.PushedAt, 0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 558 - 569, The loop that populates tagMap from m.Tags stores raw Keppel tag names, causing mismatches with CloudProfile normalized versions (underscores -> plus signs); update the tag insertion in the loop (where tagMap[t.Name] = time.Unix(...)) to normalize t.Name the same way CloudProfile versions are normalized (e.g., replace "_" with "+") before using it as the key, so lookups against referencedVersions will succeed.
🧹 Nitpick comments (2)
controllers/managedcloudprofile_controller.go (1)
648-657: Non-Keppel registries now fail GC unconditionally.The
getRegistryProvideronly returns a client for registries containing "keppel". All other OCI registries will return an error, causing GC to fail withFailedReconcileStatus.If this is intentional (Keppel-only GC support), consider:
- Documenting this limitation in the ManagedCloudProfile CRD or README
- Logging a warning/info message when GC is skipped due to unsupported registry type instead of failing
💡 Alternative: Skip GC gracefully for unsupported registries
func (r *Reconciler) getRegistryProvider(registry string) (registryClient RegistryClient, err error) { if registry == "" { return nil, errors.New("registry cannot be empty") } if strings.Contains(strings.ToLower(registry), "keppel") { return &KeppelClient{}, nil } - return nil, errors.New("no registry provider found for registry") + return nil, nil // Return nil client to indicate GC not supported }Then in
reconcileGarbageCollection:registryClient, err := r.RegistryProviderFunc(updates.Source.OCI.Registry) -if err != nil { +if err != nil || registryClient == nil { + if registryClient == nil { + log.V(1).Info("GC not supported for registry, skipping", "registry", updates.Source.OCI.Registry) + continue + } return r.failWithStatusUpdate(ctx, mcp, ...) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller.go` around lines 648 - 657, getRegistryProvider currently returns a KeppelClient only when the registry string contains "keppel" and otherwise returns an error, which causes reconcileGarbageCollection to fail for non-Keppel registries; update getRegistryProvider to either (A) document Keppel-only support in the ManagedCloudProfile CRD/README and change callers (e.g., reconcileGarbageCollection) to treat a “no provider” error as a non-fatal skip with an info/warning log, or (B) implement and return appropriate RegistryClient implementations for other OCI registries; locate and update getRegistryProvider, reconcileGarbageCollection, and any use sites checking the RegistryClient/KeppelClient to ensure unsupported registries are handled by logging and skipping GC instead of returning FailedReconcileStatus.controllers/managedcloudprofile_controller_test.go (1)
428-516: Clarify test intent: GC failure mode vs. preservation logic.This test expects
FailedReconcileStatusbecauseregistryAddr(127.0.0.1:48081) doesn't contain "keppel", causinggetRegistryProviderto fail. The test validates that:
- CloudProfile updates succeed even when GC fails
- Versions from OCI source (
1.0.1+abc) are added despite GC failureConsider adding a brief comment explaining that this tests graceful handling of GC provider failures rather than pure preservation logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controllers/managedcloudprofile_controller_test.go` around lines 428 - 516, The test "preserves old machine image versions referenced by Shoot worker pools" is exercising graceful handling of garbage-collection provider failures (getRegistryProvider) due to registryAddr not containing "keppel", so add a concise inline comment at the start of that It(...) block (or directly above the Eventually checks) stating that the test intentionally triggers a GC provider error (hence expecting FailedReconcileStatus) and verifies two things: CloudProfile updates still occur and the OCI-sourced version ("1.0.1+abc") is preserved/added despite the GC failure; mention registryAddr and getRegistryProvider by name to make intent clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.golangci.yaml:
- Around line 96-97: The forbidigo rule's regex currently matches the v3 module
too; update the forbidigo package pattern from the broad prefix
(^github\.com/coreos/go-oidc) to an exact match (^github\.com/coreos/go-oidc$)
in the .golangci.yaml rule so only the legacy module is blocked and
github.com/coreos/go-oidc/v3 is allowed; keep the existing message about
replacing with github.com/coreos/go-oidc/v3.
---
Duplicate comments:
In `@controllers/managedcloudprofile_controller.go`:
- Around line 486-524: fetchKeppelTags is creating an anonymous HTTP client and
calls registryBaseURL with a hardcoded insecure=false, so OCI credentials and
Insecure TLS settings from the source are ignored; update fetchKeppelTags to
accept the OCI config or a prepared registry client (e.g., pass in the source's
OCI Insecure flag and auth/credentials or a RegistryClient instance), use that
config when calling registryBaseURL (instead of the hardcoded false), and use
the same auth/TLS settings when building the request/HTTP client (or delegate
the call to RegistryClient.GetTags/keppelURL builder) so authenticated and
Insecure TLS registry access works correctly (refer to fetchKeppelTags,
registryBaseURL, keppelURL, RegistryClient.GetTags, and the
http.Client/TLSClientConfig creation in the diff).
- Around line 558-569: The loop that populates tagMap from m.Tags stores raw
Keppel tag names, causing mismatches with CloudProfile normalized versions
(underscores -> plus signs); update the tag insertion in the loop (where
tagMap[t.Name] = time.Unix(...)) to normalize t.Name the same way CloudProfile
versions are normalized (e.g., replace "_" with "+") before using it as the key,
so lookups against referencedVersions will succeed.
---
Nitpick comments:
In `@controllers/managedcloudprofile_controller_test.go`:
- Around line 428-516: The test "preserves old machine image versions referenced
by Shoot worker pools" is exercising graceful handling of garbage-collection
provider failures (getRegistryProvider) due to registryAddr not containing
"keppel", so add a concise inline comment at the start of that It(...) block (or
directly above the Eventually checks) stating that the test intentionally
triggers a GC provider error (hence expecting FailedReconcileStatus) and
verifies two things: CloudProfile updates still occur and the OCI-sourced
version ("1.0.1+abc") is preserved/added despite the GC failure; mention
registryAddr and getRegistryProvider by name to make intent clear.
In `@controllers/managedcloudprofile_controller.go`:
- Around line 648-657: getRegistryProvider currently returns a KeppelClient only
when the registry string contains "keppel" and otherwise returns an error, which
causes reconcileGarbageCollection to fail for non-Keppel registries; update
getRegistryProvider to either (A) document Keppel-only support in the
ManagedCloudProfile CRD/README and change callers (e.g.,
reconcileGarbageCollection) to treat a “no provider” error as a non-fatal skip
with an info/warning log, or (B) implement and return appropriate RegistryClient
implementations for other OCI registries; locate and update getRegistryProvider,
reconcileGarbageCollection, and any use sites checking the
RegistryClient/KeppelClient to ensure unsupported registries are handled by
logging and skipping GC instead of returning FailedReconcileStatus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4405288-0bf9-4e4e-a47f-3eefb4e64f2f
📒 Files selected for processing (8)
.github/workflows/checks.yaml.github/workflows/ci.yaml.github/workflows/codeql.yaml.golangci.yamlcloudprofilesync/source.gocontrollers/managedcloudprofile_controller.gocontrollers/managedcloudprofile_controller_test.gocontrollers/suite_test.go
💤 Files with no reviewable changes (1)
- cloudprofilesync/source.go
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/codeql.yaml
- .github/workflows/checks.yaml
anton-paulovich
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores