Skip to content

fix: avoid omitting boolean values when serializing#389

Open
DavidLindbom wants to merge 1 commit into
streamnative:mainfrom
DavidLindbom:fix-boolean-omitempty
Open

fix: avoid omitting boolean values when serializing#389
DavidLindbom wants to merge 1 commit into
streamnative:mainfrom
DavidLindbom:fix-boolean-omitempty

Conversation

@DavidLindbom
Copy link
Copy Markdown

Fixes #374

Motivation

When setting topicAutoCreationConfig.allow: false on a PulsarNamespace resource the operator updates the object with the field removed which causes our gitops tool to reapply the field. Over and over again.

Modifications

Removed omitempty for all boolean fields.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • controllers/pulsartopic_boolean_persistence_test.go

Documentation

  • no-need-doc

The patch is not a functional change.

@DavidLindbom DavidLindbom requested review from a team as code owners March 28, 2026 13:51
@github-actions github-actions Bot added the no-need-doc This pr does not need any document label Mar 28, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a GitOps reconciliation loop caused by omitempty dropping false boolean values during JSON serialization, leading the operator to unintentionally remove explicitly-set boolean fields (e.g., tlsAllowInsecureConnection: false) from resources.

Changes:

  • Removed omitempty from various boolean JSON tags across CRD Go types to ensure false values are serialized.
  • Added a Ginkgo test to assert false boolean values are preserved in JSON output for PulsarTopicStatus.GeoReplicationEnabled.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 24 comments.

Show a summary per file
File Description
controllers/pulsartopic_boolean_persistence_test.go Adds a serialization regression test for a false status boolean.
api/v1alpha1/pulsartopic_types.go Removes omitempty from several boolean fields (spec + status).
api/v1alpha1/pulsarsink_types.go Removes omitempty from boolean spec fields.
api/v1alpha1/pulsarnamespace_types.go Removes omitempty from multiple boolean fields (including nested configs/status).
api/v1alpha1/pulsarfunction_types.go Removes omitempty from multiple boolean spec fields.
api/v1alpha1/pulsarconnection_types.go Removes omitempty from connection TLS boolean flags to preserve false.
api/v1alpha1/computeworkspace_types.go Removes omitempty from a boolean pointer field.
api/v1alpha1/computeflinkdeployment_types.go Removes omitempty from a boolean field in restore strategy config.
api/v1alpha1/common.go Removes omitempty from shared config boolean fields so false is serialized.
api/v1alpha1/apikey_types.go Removes omitempty from an optional boolean pointer field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// Deduplication controls whether to enable message deduplication for the topic.
// +optional
Deduplication *bool `json:"deduplication,omitempty"`
Deduplication *bool `json:"deduplication"`
Comment on lines 194 to 198
// SchemaValidationEnforced determines whether schema validation is enforced for the topic.
// When enabled, only messages that conform to the topic's schema will be accepted.
// +optional
SchemaValidationEnforced *bool `json:"schemaValidationEnforced,omitempty"`
SchemaValidationEnforced *bool `json:"schemaValidationEnforced"`

// Active determines whether delayed delivery is enabled for the topic
// +optional
Active *bool `json:"active,omitempty"`
Active *bool `json:"active"`
// DeleteWhileInactive specifies whether to delete topics while they are inactive
// +optional
DeleteWhileInactive *bool `json:"deleteWhileInactive,omitempty"`
DeleteWhileInactive *bool `json:"deleteWhileInactive"`
// Deduplication controls whether to enable message deduplication for the namespace.
// +optional
Deduplication *bool `json:"deduplication,omitempty"`
Deduplication *bool `json:"deduplication"`
Comment on lines 309 to 313
// IsAllowAutoUpdateSchema specifies whether to allow automatic schema updates.
// When enabled, producers can automatically update schemas without manual approval.
// +optional
IsAllowAutoUpdateSchema *bool `json:"isAllowAutoUpdateSchema,omitempty"`
IsAllowAutoUpdateSchema *bool `json:"isAllowAutoUpdateSchema"`

Comment on lines 314 to 318
// ValidateProducerName specifies whether to validate producer names.
// When enabled, producer names must follow specific naming conventions.
// +optional
ValidateProducerName *bool `json:"validateProducerName,omitempty"`
ValidateProducerName *bool `json:"validateProducerName"`

// CleanupSubscription is the flag to indicate whether the subscription should be cleaned up when the function is deleted
// +optional
CleanupSubscription *bool `json:"cleanupSubscription,omitempty"`
CleanupSubscription *bool `json:"cleanupSubscription"`
// RetainOrdering is the flag to indicate whether the function should retain ordering
// +optional
RetainOrdering *bool `json:"retainOrdering,omitempty"`
RetainOrdering *bool `json:"retainOrdering"`
// RetainKeyOrdering is the flag to indicate whether the function should retain key ordering
// +optional
RetainKeyOrdering *bool `json:"retainKeyOrdering,omitempty"`
RetainKeyOrdering *bool `json:"retainKeyOrdering"`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-need-doc This pr does not need any document

Projects

None yet

Development

Successfully merging this pull request may close these issues.

omitEmpty on tlsAllowInsecureConnection causes infinite loop when value is false

3 participants