Skip to content

Mark Pipeline.PipelineDefinition as is_document#355

Open
gustavodiaz7722 wants to merge 1 commit into
aws-controllers-k8s:mainfrom
gustavodiaz7722:fix/pipeline-definition-is-document
Open

Mark Pipeline.PipelineDefinition as is_document#355
gustavodiaz7722 wants to merge 1 commit into
aws-controllers-k8s:mainfrom
gustavodiaz7722:fix/pipeline-definition-is-document

Conversation

@gustavodiaz7722

Copy link
Copy Markdown
Member

Summary

Adds is_document: true annotation to Pipeline.PipelineDefinition field in generator.yaml.

Problem

The Pipeline.PipelineDefinition field accepts an Amazon SageMaker Pipeline JSON definition document. Without the is_document annotation, the controller uses strict string comparison which causes false drift detection and infinite reconciliation loops when the API returns reformatted (but semantically equivalent) JSON.

Changes

  • Added is_document: true to Pipeline.PipelineDefinition in generator.yaml
  • Regenerated controller code - delta.go now uses DocumentEqual for semantic comparison

Testing

  • go build ./cmd/controller - compiles cleanly
  • Existing e2e test (pipeline.yaml fixture) already exercises the pipelineDefinition field with JSON content

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-prow ack-prow Bot requested review from jlbutler, michaelhtm and sapphirew June 19, 2026 03:23
@ack-prow

ack-prow Bot commented Jun 19, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gustavodiaz7722
Once this PR has been reviewed and has the lgtm label, please assign jlbutler for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gustavodiaz7722

Copy link
Copy Markdown
Member Author

/label release/patch

@ack-prow ack-prow Bot added the release/patch Indicates this PR should trigger a patch version release on merge. label Jun 19, 2026
@gustavodiaz7722 gustavodiaz7722 force-pushed the fix/pipeline-definition-is-document branch from ae9840f to c3445cf Compare June 19, 2026 19:38
@gustavodiaz7722

Copy link
Copy Markdown
Member Author

Changes look good to me, waiting on review from team

// +kubebuilder:rbac:groups=sagemaker.services.k8s.aws,resources=domains,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=sagemaker.services.k8s.aws,resources=domains/status,verbs=get;update;patch

var lateInitializeFieldNames = []string{"AppNetworkAccessType", "DefaultUserSettings.CodeEditorAppSettings", "DefaultUserSettings.CustomFileSystemConfigs", "DefaultUserSettings.CustomPosixUserConfig", "DefaultUserSettings.DefaultLandingURI", "DefaultUserSettings.JupyterLabAppSettings", "DefaultUserSettings.JupyterServerAppSettings", "DefaultUserSettings.KernelGatewayAppSettings", "DefaultUserSettings.RStudioServerProAppSettings", "DefaultUserSettings.SecurityGroups", "DefaultUserSettings.SharingSettings", "DefaultUserSettings.SpaceStorageSettings", "DefaultUserSettings.StudioWebPortal", "DefaultUserSettings.TensorBoardAppSettings"}

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.

@gustavodiaz7722 any idea why these are changing?

@gustavodiaz7722 gustavodiaz7722 force-pushed the fix/pipeline-definition-is-document branch from c3445cf to 6738fa7 Compare June 22, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/patch Indicates this PR should trigger a patch version release on merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants