Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[runtime] Exclude default values and empty fields during resource adoption when using the annotation approach #2230

Open
adriananeci opened this issue Dec 13, 2024 · 3 comments
Labels
adoption-annotation Issues or PRs related to ACK Adoption by Annotation feature service/s3 Indicates issues or PRs that are related to s3-controller.

Comments

@adriananeci
Copy link

Is your feature request related to a problem?

When adopting an S3 bucket using the annotation based mechanism(services.k8s.aws/adoption-fields and services.k8s.aws/adoption-policy: adopt annotations), the ACK object's spec is populated with all available options even if many of them are empty or have the default values.

Is it possible to populate the spec with only non-default and non-empty fields during adoption?

Describe the solution you'd like
A description of what you want to happen.

Describe alternatives you've considered
A description of any alternative solutions or features you've considered.

@michaelhtm michaelhtm added the adoption-annotation Issues or PRs related to ACK Adoption by Annotation feature label Dec 13, 2024
@michaelhtm michaelhtm added the service/s3 Indicates issues or PRs that are related to s3-controller. label Dec 17, 2024
@michaelhtm
Copy link
Member

This issue seems to be happening specifically in s3. Will have a patch for it soon

ack-prow bot pushed a commit to aws-controllers-k8s/s3-controller that referenced this issue Dec 17, 2024
Issue [#2230](aws-controllers-k8s/community#2230)

Description of changes:
This PR is ensuring the controller does not assign default empty
values to the resource during a read operation

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

Hey @adriananeci, after looking into this further, we noticed that this could cause some inconsistencies between the different controllers.
It's not very straight forward figuring out what are considered empty/default fields (eg, integers, booleans, etc.)
Also i just wanted to understand if there are any issues with having empty fields..

@adriananeci
Copy link
Author

adriananeci commented Dec 19, 2024

Hey @michaelhtm,

We currently create ACK objects via some helm charts and have objects defined to something like

apiVersion: s3.services.k8s.aws/v1alpha1
kind: Bucket
metadata:
  name: {{ $bucketName }}
  namespace: {{ $namespace }}
  labels:
  {{- include "storage.labels" . | nindent 4 }}
  annotations:
    services.k8s.aws/region: {{ .Values.global.location }}
{{- if .Values.storage.adoptExistingResources }}
    services.k8s.aws/adoption-fields: |
      {
        "name": "{{ $bucketName }}"
      }
    services.k8s.aws/adoption-policy: adopt
{{- else }}
spec:
  name: {{ $bucketName }}
  lifecycle:
    rules:
    - abortIncompleteMultipartUpload:
        daysAfterInitiation: {{ .Values.storage.daysAfterInitiation }}
      id: "delete expired multi part uploads"
      status: "Enabled"
      prefix: ""
  publicAccessBlock: {{ .Values.storage.publicAccessBlock | toYaml | nindent 4 }}
  tagging:
    tagSet:
    - key: Name
      value: {{ $bucketName }}
    - key: AckName
      value: {{ $bucketName }}
    - key: kubernetes.io/cluster/{{ $clusterFullName }}
      value: owned
    {{- include "storage.tags" . | nindent 4 -}}
{{- end }}

We are planning to adopt some existing AWS resources by leveraging the adoption annotation feature and then continue to reconcile/adjust the adopted ACK resource via the helm charts.

In order to make sure we are not missing any ACK objects setting in their spec definition, I want do a diff/comparison between the ACK object spec as populated by the controller and the ACK object spec defined in helm template.

Given currently there are many fields that are empty or have a default value when an ACK object is adopted, it is pretty hard to identify if there is a gap between the object template as defined into the helm chart and the one populated during the adoption during the diff process.

For an adopted S3 bucket in particular, I've noticed many fields that are empty or have default values, like:

spec:
  accelerate: {}
  acl: private|bucket-owner-read|bucket-owner-full-control
  cors: {}
  encryption:
    rules:
    - applyServerSideEncryptionByDefault:
        sseAlgorithm: AES256
      bucketKeyEnabled: false
  grantFullControl: id=xxxxx
  grantRead: ""
  grantReadACP: ""
  grantWrite: ""
  grantWriteACP: ""
  lifecycle: {}
  logging: {}
  notification: {}
  ownershipControls:
    rules:
    - objectOwnership: BucketOwnerEnforced
  requestPayment:
    payer: BucketOwner

In order to replicate it, you can manually create an S3 bucket with the default settings via the AWS console or aws cli and then try to adopt it. Then create a new ACK S3 bucket object with only the mandatory fields (e.g. name) , and you'll notice the diff in terms of spec fields between these 2 ACK objects, even if the AWS created resources have the same settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adoption-annotation Issues or PRs related to ACK Adoption by Annotation feature service/s3 Indicates issues or PRs that are related to s3-controller.
Projects
None yet
Development

No branches or pull requests

2 participants