Skip to content

Commit

Permalink
fix: resourceRef should be a required property (#411)
Browse files Browse the repository at this point in the history
## Description

Closes open-component-model/ocm-project#66


## What type of PR is this? (check all applicable)

- [ ] πŸ• Feature
- [ ] πŸ› Bug Fix
- [ ] πŸ“ Documentation Update
- [ ] 🎨 Style
- [ ] πŸ§‘β€πŸ’» Code Refactor
- [ ] πŸ”₯ Performance Improvements
- [ ] βœ… Test
- [ ] πŸ€– Build
- [ ] πŸ” CI
- [ ] πŸ“¦ Chore (Release)
- [ ] ⏩ Revert

## Related Tickets & Documents

<!-- 
Please use this format link issue numbers: Fixes #123

https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->
- Related Issue # (issue)
- Closes # (issue)
- Fixes # (issue)
> Remove if not applicable

## Screenshots

<!-- Visual changes require screenshots -->


## Added tests?

- [ ] πŸ‘ yes
- [ ] πŸ™… no, because they aren't needed
- [ ] πŸ™‹ no, because I need help
- [ ] Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also list any relevant details
for your test configuration


## Added to documentation?

- [ ] πŸ“œ README.md
- [ ] πŸ™… no documentation needed

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream
modules
  • Loading branch information
Skarlso authored May 3, 2024
1 parent 1abed0c commit d6ff6a2
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 15 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/reference_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
type ObjectReference struct {
meta.NamespacedObjectKindReference `json:",inline"`

// ResourceRef defines what resource to fetch.
// +optional
ResourceRef *ResourceReference `json:"resourceRef,omitempty"`
}
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ func (in Resource) GetRequeueAfter() time.Duration {

// GetReferencePath returns the component reference path for the Resource.
func (in Resource) GetReferencePath() []ocmmetav1.Identity {
if in.Spec.SourceRef.ResourceRef == nil {
return nil
}

return in.Spec.SourceRef.ResourceRef.ReferencePath
}

Expand Down
2 changes: 2 additions & 0 deletions config/crd/bases/delivery.ocm.software_configurations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down Expand Up @@ -217,6 +218,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/delivery.ocm.software_fluxdeployers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down
2 changes: 2 additions & 0 deletions config/crd/bases/delivery.ocm.software_localizations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down Expand Up @@ -217,6 +218,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/delivery.ocm.software_resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ spec:
acts as LocalObjectReference.
type: string
resourceRef:
description: ResourceRef defines what resource to fetch.
properties:
extraIdentity:
additionalProperties:
Expand Down
19 changes: 14 additions & 5 deletions controllers/mutation_reconcile_looper.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,17 @@ func (m *MutationReconcileLooper) localize(
mutationSpec *v1alpha1.MutationSpec,
data, configObj []byte,
) (string, error) {
log := log.FromContext(ctx)
logger := log.FromContext(ctx)

cv, err := m.getComponentVersion(ctx, mutationSpec.ConfigRef)
if err != nil {
return "", fmt.Errorf("failed to get component version: %w", err)
}

refPath := mutationSpec.ConfigRef.ResourceRef.ReferencePath
var refPath []ocmmetav1.Identity
if mutationSpec.ConfigRef.ResourceRef != nil {
refPath = mutationSpec.ConfigRef.ResourceRef.ReferencePath
}

virtualFS, err := osfs.NewTempFileSystem()
if err != nil {
Expand Down Expand Up @@ -225,7 +228,7 @@ func (m *MutationReconcileLooper) localize(
}

if len(rules) == 0 {
log.Info("no rules generated from the available config data; the generate snapshot will have no modifications")
logger.Info("no rules generated from the available config data; the generate snapshot will have no modifications")
}

if err := localize.Substitute(rules, virtualFS); err != nil {
Expand Down Expand Up @@ -297,6 +300,10 @@ func (m *MutationReconcileLooper) fetchDataFromComponentVersion(ctx context.Cont
return nil, fmt.Errorf("failed to create authenticated client: %w", err)
}

if obj.ResourceRef == nil {
return nil, fmt.Errorf("no resource ref found for %s", key)
}

resource, _, _, err := m.OCMClient.GetResource(ctx, octx, componentVersion, obj.ResourceRef)
if err != nil {
return nil, fmt.Errorf("failed to fetch resource from component version: %w", err)
Expand Down Expand Up @@ -743,8 +750,10 @@ func (m *MutationReconcileLooper) getIdentity(ctx context.Context, obj *v1alpha1
id = ocmmetav1.Identity{
v1alpha1.ComponentNameKey: cv.Status.ComponentDescriptor.ComponentDescriptorRef.Name,
v1alpha1.ComponentVersionKey: cv.Status.ComponentDescriptor.Version,
v1alpha1.ResourceNameKey: obj.ResourceRef.Name,
v1alpha1.ResourceVersionKey: obj.ResourceRef.Version,
}
if obj.ResourceRef != nil {
id[v1alpha1.ResourceNameKey] = obj.ResourceRef.Name
id[v1alpha1.ResourceVersionKey] = obj.ResourceRef.Version
}
default:
// if kind is not ComponentVersion, then fetch resource using dynamic client
Expand Down
4 changes: 4 additions & 0 deletions controllers/resource_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ func (r *ResourceReconciler) Reconcile(
return result, nil
}

if obj.Spec.SourceRef.ResourceRef == nil {
return ctrl.Result{}, fmt.Errorf("resource requires resource ref")
}

patchHelper := patch.NewSerialPatcher(obj, r.Client)

// Always attempt to patch the object and status after each reconciliation.
Expand Down
21 changes: 11 additions & 10 deletions controllers/resourcepipeline_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,14 @@ func (r *ResourcePipelineReconciler) Reconcile(
return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

if obj.Spec.SourceRef.ResourceRef == nil {
return ctrl.Result{}, fmt.Errorf("resource reference is needed to fetch the pipeline objects")
}

patchHelper := patch.NewSerialPatcher(obj, r.Client)

// Always attempt to patch the object and status after each reconciliation.
defer func() {
// Patching has not been set up, or the controller errored earlier.
if patchHelper == nil {
return
}

if condition := conditions.Get(obj, meta.StalledCondition); condition != nil &&
condition.Status == metav1.ConditionTrue {
conditions.Delete(obj, meta.ReconcilingCondition)
Expand Down Expand Up @@ -510,13 +509,15 @@ func (r *ResourcePipelineReconciler) getIdentity(
id = ocmmetav1.Identity{
v1alpha1.ComponentNameKey: cv.Status.ComponentDescriptor.ComponentDescriptorRef.Name,
v1alpha1.ComponentVersionKey: cv.Status.ComponentDescriptor.Version,
v1alpha1.ResourceNameKey: obj.ResourceRef.Name,
v1alpha1.ResourceVersionKey: obj.ResourceRef.Version,
}

// apply the extra identity fields if provided
for k, v := range obj.ResourceRef.ExtraIdentity {
id[k] = v
if obj.ResourceRef != nil {
id[v1alpha1.ResourceNameKey] = obj.ResourceRef.Name
id[v1alpha1.ResourceVersionKey] = obj.ResourceRef.Version
// apply the extra identity fields if provided
for k, v := range obj.ResourceRef.ExtraIdentity {
id[k] = v
}
}

return id, err
Expand Down

0 comments on commit d6ff6a2

Please sign in to comment.