Skip to content

Commit

Permalink
pkg/manifest: Add 'overrides' handling
Browse files Browse the repository at this point in the history
Centralize this aspect of manifest filtering, so it can be shared
between the cluster-version operator (where it currently lives [1])
and 'oc' (where we want manifest extraction to understand this sort of
condition [2]).

[1]: https://github.com/openshift/cluster-version-operator/blob/cce97c25292b263a5a549b2bb8ce82ed7f2d4923/pkg/cvo/sync_worker.go#L959-L987
[2]: https://issues.redhat.com/browse/OTA-559
  • Loading branch information
wking committed Apr 20, 2023
1 parent a704a57 commit 087d2eb
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 9 deletions.
41 changes: 36 additions & 5 deletions pkg/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,12 @@ func (m *Manifest) UnmarshalJSON(in []byte) error {
if !ok {
return fmt.Errorf("expected manifest to decode into *unstructured.Unstructured, got %T", ud)
}
m.GVK = ud.GroupVersionKind()
m.Obj = ud
return m.populateFromObj()
}

func (m *Manifest) populateFromObj() error {
m.GVK = m.Obj.GroupVersionKind()
m.id = resourceId{
Group: m.GVK.Group,
Kind: m.GVK.Kind,
Expand Down Expand Up @@ -170,8 +174,8 @@ func checkFeatureSets(requiredFeatureSet string, annotations map[string]string)
// processing by cluster version operator. Pointer arguments can be set nil to avoid excluding based on that
// filter. For example, setting profile non-nil and capabilities nil will return an error if the manifest's
// profile does not match, but will never return an error about capability issues.
func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus) error {
return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, false)
func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride) error {
return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, overrides, false)
}

// IncludeAllowUnknownCapabilities returns an error if the manifest fails an inclusion filter and should be excluded from
Expand All @@ -181,7 +185,7 @@ func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string
// to capabilities filtering. When set to true a manifest will not be excluded simply because it contains an unknown
// capability. This is necessary to allow updates to an OCP version containing newly defined capabilities.
func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, requiredFeatureSet *string, profile *string,
capabilities *configv1.ClusterVersionCapabilitiesStatus, allowUnknownCapabilities bool) error {
capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, allowUnknownCapabilities bool) error {

annotations := m.Obj.GetAnnotations()
if annotations == nil {
Expand Down Expand Up @@ -213,7 +217,34 @@ func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, re

// If there is no capabilities defined in a release then we do not need to check presence of capabilities in the manifest
if capabilities != nil {
return checkResourceEnablement(annotations, capabilities, allowUnknownCapabilities)
err := checkResourceEnablement(annotations, capabilities, allowUnknownCapabilities)
if err != nil {
return err
}
}

if override := m.getOverrideForManifest(overrides); override != nil && override.Unmanaged {
return fmt.Errorf("overridden")
}

return nil
}

// getOverrideForManifest returns the override when override exists and nil otherwise.
func (m *Manifest) getOverrideForManifest(overrides []configv1.ComponentOverride) *configv1.ComponentOverride {
for _, override := range overrides {
namespace := override.Namespace
if m.id.Namespace == "" {
namespace = "" // cluster-scoped objects don't have namespace.
}
if m.id.equal(resourceId{
Group: override.Group,
Kind: override.Kind,
Name: override.Name,
Namespace: namespace,
}) {
return &override
}
}
return nil
}
Expand Down
56 changes: 52 additions & 4 deletions pkg/manifest/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ func Test_include(t *testing.T) {
profile *string
annotations map[string]interface{}
caps *configv1.ClusterVersionCapabilitiesStatus
overrides []configv1.ComponentOverride

expected error
}{
Expand Down Expand Up @@ -765,6 +766,44 @@ func Test_include(t *testing.T) {
EnabledCapabilities: []configv1.ClusterVersionCapability{"cap1", "cap2", "cap3"},
},
},
{
name: "unrelated override",
annotations: map[string]interface{}{},
overrides: []configv1.ComponentOverride{
{
Kind: "Pod",
Name: "my-pod",
Namespace: "my-namespace",
Unmanaged: true,
},
},
},
{
name: "override, but managed",
annotations: map[string]interface{}{},
overrides: []configv1.ComponentOverride{
{
Group: "apps",
Kind: "Deployment",
Name: "my-deployment",
Namespace: "my-namespace",
},
},
},
{
name: "unmanaged override",
annotations: map[string]interface{}{},
overrides: []configv1.ComponentOverride{
{
Group: "apps",
Kind: "Deployment",
Name: "my-deployment",
Namespace: "my-namespace",
Unmanaged: true,
},
},
expected: fmt.Errorf("overridden"),
},
{
name: "all nil",
profile: nil,
Expand All @@ -774,19 +813,27 @@ func Test_include(t *testing.T) {
}

for _, tt := range tests {
metadata := map[string]interface{}{}
metadata := map[string]interface{}{
"name": "my-deployment",
"namespace": "my-namespace",
}
t.Run(tt.name, func(t *testing.T) {
if tt.annotations != nil {
metadata["annotations"] = tt.annotations
}
m := Manifest{
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": metadata,
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": metadata,
},
},
}
err := m.Include(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps)
err := m.populateFromObj()
assert.Equal(t, nil, err)

err = m.Include(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides)
assert.Equal(t, tt.expected, err)
})
}
Expand All @@ -801,6 +848,7 @@ func TestIncludeAllowUnknownCapabilities(t *testing.T) {
profile *string
annotations map[string]interface{}
caps *configv1.ClusterVersionCapabilitiesStatus
overrides []configv1.ComponentOverride
allowUnknown bool

expected error
Expand Down Expand Up @@ -844,7 +892,7 @@ func TestIncludeAllowUnknownCapabilities(t *testing.T) {
},
},
}
err := m.IncludeAllowUnknownCapabilities(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.allowUnknown)
err := m.IncludeAllowUnknownCapabilities(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides, tt.allowUnknown)
assert.Equal(t, tt.expected, err)
})
}
Expand Down

0 comments on commit 087d2eb

Please sign in to comment.