Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions docs/concepts/large-bundle-support.md
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest moving this file elsewhere. The concepts docs are user-facing docs that describe functions/behaviors that users need to be aware of and directly interact with.

The typical doc flow is to put everything in drafts and then do a separate "promotion" out of drafts.

We probably also now need a specific process for design doc organization.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah - let's do that. Or maybe even drop the docs from this PR. I've just put up a doc PR. Maybe we could iterate there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest moving this file elsewhere. The concepts docs are user-facing docs that describe functions/behaviors that users need to be aware of and directly interact with.

Sure, but let's do that in a separate - follow up PR.

Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,19 @@ follow for consistency and safe lifecycle management.

Recommended conventions:

1. **Immutability**: Secrets should set `immutable: true`. Because COS phases
1. **Secret type**: Secrets should use the dedicated type
`olm.operatorframework.io/object-data` to distinguish them from user-created
Secrets and enable easy identification. The system always sets this type on
Secrets it creates. The reconciler does not enforce the type when resolving
refs — Secrets with any type are accepted — but producers should set it for
consistency.

2. **Immutability**: Secrets should set `immutable: true`. Because COS phases
are immutable, the content backing a ref should not change after creation.
Mutable referenced Secrets are not rejected, but modifying them after the
COS is created leads to undefined behavior.

2. **Owner references**: Referenced Secrets should carry an ownerReference to
3. **Owner references**: Referenced Secrets should carry an ownerReference to
the COS so that Kubernetes garbage collection removes them when the COS is
deleted:
```yaml
Expand All @@ -159,7 +166,7 @@ Recommended conventions:
Secret when the COS is deleted. The reconciler does not delete referenced
Secrets itself.

3. **Revision label**: A label identifying the owning revision aids discovery,
4. **Revision label**: A label identifying the owning revision aids discovery,
debugging, and bulk cleanup:
```
olm.operatorframework.io/revision-name: <COS-name>
Expand Down Expand Up @@ -237,6 +244,7 @@ metadata:
uid: <revision-uid>
controller: true
immutable: true
type: olm.operatorframework.io/object-data
data:
service-account: <base64(JSON ServiceAccount manifest)>
cluster-role: <base64(JSON ClusterRole manifest)>
Expand All @@ -255,6 +263,7 @@ metadata:
uid: <revision-uid>
controller: true
immutable: true
type: olm.operatorframework.io/object-data
data:
my-crd: <base64(JSON CRD manifest)>
---
Expand All @@ -272,6 +281,7 @@ metadata:
uid: <revision-uid>
controller: true
immutable: true
type: olm.operatorframework.io/object-data
data:
deployment: <base64(JSON Deployment manifest)>
```
Expand Down Expand Up @@ -653,7 +663,7 @@ rollout semantics are unchanged.
| **Crash safety** | 3-step: Secrets → COS → patch ownerRefs; orphan cleanup via revision label | 2-step: COS → Secrets with ownerRefs; simpler but reconciler may see missing Secrets temporarily |
| **Flexibility** | Mixed inline/ref per object within the same phase is possible | All-or-nothing — either all phases inline or all externalized |
| **Storage efficiency** | Per-object compression misses cross-object redundancy; potentially more Secrets created in edge cases | Better compression from cross-phase redundancy; fewer Secrets |
| **Resource type** | Secret only | Secret only (with dedicated type) |
| **Resource type** | Secret with dedicated type `olm.operatorframework.io/object-data` | Secret with dedicated type `olm.operatorframework.io/revision-phase-data` |
| **Phases structure** | Unchanged — phases array preserved as-is; only individual objects gain a new resolution path | Replaced at the top level — phases field swapped for phasesRef |
| **Content addressability** | Content hash as Secret data key — key changes when content changes | Content hash embedded in Secret name — detects changes without fetching contents |

Expand Down
1 change: 1 addition & 0 deletions internal/operator-controller/applier/secretpacker.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func (p *SecretPacker) newSecret(data map[string][]byte) corev1.Secret {
},
},
Immutable: ptr.To(true),
Type: labels.SecretTypeObjectData,
Data: data,
}
}
Expand Down
1 change: 1 addition & 0 deletions internal/operator-controller/applier/secretpacker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestSecretPacker_Pack(t *testing.T) {
assert.True(t, strings.HasPrefix(result.Secrets[0].Name, "my-ext-3-"), "Secret name should be content-addressable with revision prefix")
assert.Equal(t, "olmv1-system", result.Secrets[0].Namespace)
assert.True(t, *result.Secrets[0].Immutable)
assert.Equal(t, labels.SecretTypeObjectData, result.Secrets[0].Type)
assert.Equal(t, "my-ext-3", result.Secrets[0].Labels[labels.RevisionNameKey])
assert.Equal(t, "my-ext", result.Secrets[0].Labels[labels.OwnerNameKey])

Expand Down
7 changes: 7 additions & 0 deletions internal/operator-controller/labels/labels.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
package labels

import corev1 "k8s.io/api/core/v1"

const (
// SecretTypeObjectData is the custom Secret type used for Secrets that store
// externalized object content referenced by ClusterObjectSet ref entries.
// It distinguishes OLM-managed ref Secrets from user-created Secrets.
SecretTypeObjectData corev1.SecretType = "olm.operatorframework.io/object-data" //nolint:gosec // G101 false positive: this is a Kubernetes Secret type identifier, not a credential

// OwnerKindKey is the label key used to record the kind of the owner
// resource responsible for creating or managing a ClusterObjectSet.
OwnerKindKey = "olm.operatorframework.io/owner-kind"
Expand Down
1 change: 1 addition & 0 deletions test/e2e/features/install.feature
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ Feature: Install ClusterExtension
And ClusterObjectSet "${NAME}-1" ref Secrets are immutable
And ClusterObjectSet "${NAME}-1" ref Secrets are labeled with revision and owner
And ClusterObjectSet "${NAME}-1" ref Secrets have ownerReference to the revision
And ClusterObjectSet "${NAME}-1" ref Secrets have type "olm.operatorframework.io/object-data"

@DeploymentConfig
Scenario: deploymentConfig nodeSelector is applied to the operator deployment
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/steps/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are immutable$`, ClusterObjectSetRefSecretsAreImmutable)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets are labeled with revision and owner$`, ClusterObjectSetRefSecretsLabeled)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have ownerReference to the revision$`, ClusterObjectSetRefSecretsHaveOwnerRef)
sc.Step(`^(?i)ClusterObjectSet "([^"]+)" ref Secrets have type "([^"]+)"$`, ClusterObjectSetReferredSecretsHaveType)

sc.Step(`^(?i)resource "([^"]+)" is installed$`, ResourceAvailable)
sc.Step(`^(?i)resource "([^"]+)" is available$`, ResourceAvailable)
Expand Down Expand Up @@ -875,6 +876,28 @@ func ClusterObjectSetRefSecretsHaveOwnerRef(ctx context.Context, revisionName st
return nil
}

// ClusterObjectSetReferredSecretsHaveType verifies that all ref Secrets for the named
// ClusterObjectSet have the specified Secret type.
func ClusterObjectSetReferredSecretsHaveType(ctx context.Context, revisionName, expectedType string) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)

secrets, err := listRefSecrets(ctx, revisionName)
if err != nil {
return err
}
if len(secrets) == 0 {
return fmt.Errorf("no ref Secrets found for revision %q", revisionName)
}

for _, s := range secrets {
if string(s.Type) != expectedType {
return fmt.Errorf("secret %s/%s has type %q, expected %q", s.Namespace, s.Name, s.Type, expectedType)
}
}
return nil
}

// collectRefSecretNames returns the unique set of Secret names referenced by the ClusterObjectSet's phase objects.
func collectRefSecretNames(ctx context.Context, revisionName string) ([]string, error) {
var names []string
Expand Down
Loading