Skip to content

Commit 14972d4

Browse files
michaelhtmmichaelhtm
authored
Exclude default/empty values from spec during a read operation (#143)
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.
1 parent 64382cd commit 14972d4

File tree

1 file changed

+123
-72
lines changed

1 file changed

+123
-72
lines changed

‎pkg/resource/bucket/hook.go‎

Lines changed: 123 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -317,21 +317,27 @@ func (rm *resourceManager) addPutFieldsToSpec(
317317
// This method is not supported in every region, ignore any errors if
318318
// we attempt to describe this property in a region in which it's not
319319
// supported.
320-
if awsErr, ok := ackerr.AWSError(err); ok && (awsErr.Code() == "MethodNotAllowed" || awsErr.Code() == "UnsupportedArgument") {
321-
getAccelerateResponse = &svcsdk.GetBucketAccelerateConfigurationOutput{}
322-
} else {
320+
if awsErr, ok := ackerr.AWSError(err); !ok || (awsErr.Code() != "MethodNotAllowed" && awsErr.Code() != "UnsupportedArgument") {
323321
return err
324322
}
325323
}
326-
ko.Spec.Accelerate = rm.setResourceAccelerate(r, getAccelerateResponse)
324+
if getAccelerateResponse.Status != nil && *getAccelerateResponse.Status != "" {
325+
ko.Spec.Accelerate = rm.setResourceAccelerate(r, getAccelerateResponse)
326+
} else if (getAccelerateResponse.Status == nil || *getAccelerateResponse.Status == "") && ko.Spec.Accelerate != nil {
327+
ko.Spec.Accelerate = &svcapitypes.AccelerateConfiguration{}
328+
}
327329

328330
listAnalyticsResponse, err := rm.sdkapi.ListBucketAnalyticsConfigurationsWithContext(ctx, rm.newListBucketAnalyticsPayload(r))
329331
if err != nil {
330332
return err
331333
}
332-
ko.Spec.Analytics = make([]*svcapitypes.AnalyticsConfiguration, len(listAnalyticsResponse.AnalyticsConfigurationList))
333-
for i, analyticsConfiguration := range listAnalyticsResponse.AnalyticsConfigurationList {
334-
ko.Spec.Analytics[i] = rm.setResourceAnalyticsConfiguration(r, analyticsConfiguration)
334+
if listAnalyticsResponse != nil && len(listAnalyticsResponse.AnalyticsConfigurationList) > 0 {
335+
ko.Spec.Analytics = make([]*svcapitypes.AnalyticsConfiguration, len(listAnalyticsResponse.AnalyticsConfigurationList))
336+
for i, analyticsConfiguration := range listAnalyticsResponse.AnalyticsConfigurationList {
337+
ko.Spec.Analytics[i] = rm.setResourceAnalyticsConfiguration(r, analyticsConfiguration)
338+
}
339+
} else if (listAnalyticsResponse == nil || len(listAnalyticsResponse.AnalyticsConfigurationList) == 0) && ko.Spec.Analytics != nil {
340+
ko.Spec.Analytics = []*svcapitypes.AnalyticsConfiguration{}
335341
}
336342

337343
getACLResponse, err := rm.sdkapi.GetBucketAclWithContext(ctx, rm.newGetBucketACLPayload(r))
@@ -342,160 +348,193 @@ func (rm *resourceManager) addPutFieldsToSpec(
342348

343349
getCORSResponse, err := rm.sdkapi.GetBucketCorsWithContext(ctx, rm.newGetBucketCORSPayload(r))
344350
if err != nil {
345-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchCORSConfiguration" {
346-
getCORSResponse = &svcsdk.GetBucketCorsOutput{}
347-
} else {
351+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchCORSConfiguration" {
348352
return err
349353
}
350354
}
351-
ko.Spec.CORS = rm.setResourceCORS(r, getCORSResponse)
355+
if getCORSResponse.CORSRules != nil {
356+
ko.Spec.CORS = rm.setResourceCORS(r, getCORSResponse)
357+
} else if getCORSResponse.CORSRules == nil && ko.Spec.CORS != nil {
358+
ko.Spec.CORS = &svcapitypes.CORSConfiguration{}
359+
}
352360

353361
getEncryptionResponse, err := rm.sdkapi.GetBucketEncryptionWithContext(ctx, rm.newGetBucketEncryptionPayload(r))
354362
if err != nil {
355-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "ServerSideEncryptionConfigurationNotFoundError" {
356-
getEncryptionResponse = &svcsdk.GetBucketEncryptionOutput{
357-
ServerSideEncryptionConfiguration: &svcsdk.ServerSideEncryptionConfiguration{},
358-
}
359-
} else {
363+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "ServerSideEncryptionConfigurationNotFoundError" {
360364
return err
361365
}
366+
}
367+
if getEncryptionResponse.ServerSideEncryptionConfiguration.Rules != nil {
368+
ko.Spec.Encryption = rm.setResourceEncryption(r, getEncryptionResponse)
369+
} else if getEncryptionResponse.ServerSideEncryptionConfiguration.Rules == nil && ko.Spec.Encryption != nil {
370+
ko.Spec.Encryption = &svcapitypes.ServerSideEncryptionConfiguration{}
362371
}
363-
ko.Spec.Encryption = rm.setResourceEncryption(r, getEncryptionResponse)
364372

365373
listIntelligentTieringResponse, err := rm.sdkapi.ListBucketIntelligentTieringConfigurationsWithContext(ctx, rm.newListBucketIntelligentTieringPayload(r))
366374
if err != nil {
367375
return err
368-
}
369-
ko.Spec.IntelligentTiering = make([]*svcapitypes.IntelligentTieringConfiguration, len(listIntelligentTieringResponse.IntelligentTieringConfigurationList))
370-
for i, intelligentTieringConfiguration := range listIntelligentTieringResponse.IntelligentTieringConfigurationList {
371-
ko.Spec.IntelligentTiering[i] = rm.setResourceIntelligentTieringConfiguration(r, intelligentTieringConfiguration)
376+
}
377+
if len(listIntelligentTieringResponse.IntelligentTieringConfigurationList) > 0 {
378+
ko.Spec.IntelligentTiering = make([]*svcapitypes.IntelligentTieringConfiguration, len(listIntelligentTieringResponse.IntelligentTieringConfigurationList))
379+
for i, intelligentTieringConfiguration := range listIntelligentTieringResponse.IntelligentTieringConfigurationList {
380+
ko.Spec.IntelligentTiering[i] = rm.setResourceIntelligentTieringConfiguration(r, intelligentTieringConfiguration)
381+
}
382+
} else if len(listIntelligentTieringResponse.IntelligentTieringConfigurationList) == 0 && len(ko.Spec.IntelligentTiering) > 0 {
383+
ko.Spec.IntelligentTiering = []*svcapitypes.IntelligentTieringConfiguration{}
372384
}
373385

374386
listInventoryResponse, err := rm.sdkapi.ListBucketInventoryConfigurationsWithContext(ctx, rm.newListBucketInventoryPayload(r))
375387
if err != nil {
376388
return err
377-
}
378-
ko.Spec.Inventory = make([]*svcapitypes.InventoryConfiguration, len(listInventoryResponse.InventoryConfigurationList))
379-
for i, inventoryConfiguration := range listInventoryResponse.InventoryConfigurationList {
380-
ko.Spec.Inventory[i] = rm.setResourceInventoryConfiguration(r, inventoryConfiguration)
389+
} else {
390+
ko.Spec.Inventory = make([]*svcapitypes.InventoryConfiguration, len(listInventoryResponse.InventoryConfigurationList))
391+
for i, inventoryConfiguration := range listInventoryResponse.InventoryConfigurationList {
392+
ko.Spec.Inventory[i] = rm.setResourceInventoryConfiguration(r, inventoryConfiguration)
393+
}
381394
}
382395

383396
getLifecycleResponse, err := rm.sdkapi.GetBucketLifecycleConfigurationWithContext(ctx, rm.newGetBucketLifecyclePayload(r))
384397
if err != nil {
385-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchLifecycleConfiguration" {
386-
getLifecycleResponse = &svcsdk.GetBucketLifecycleConfigurationOutput{}
387-
} else {
398+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchLifecycleConfiguration" {
388399
return err
389400
}
401+
}
402+
if getLifecycleResponse.Rules != nil {
403+
ko.Spec.Lifecycle = rm.setResourceLifecycle(r, getLifecycleResponse)
404+
} else if getLifecycleResponse.Rules == nil && ko.Spec.Lifecycle != nil {
405+
ko.Spec.Lifecycle = &svcapitypes.BucketLifecycleConfiguration{}
390406
}
391-
ko.Spec.Lifecycle = rm.setResourceLifecycle(r, getLifecycleResponse)
392407

393408
getLoggingResponse, err := rm.sdkapi.GetBucketLoggingWithContext(ctx, rm.newGetBucketLoggingPayload(r))
394409
if err != nil {
395410
return err
396411
}
397-
ko.Spec.Logging = rm.setResourceLogging(r, getLoggingResponse)
412+
if getLoggingResponse.LoggingEnabled != nil {
413+
ko.Spec.Logging = rm.setResourceLogging(r, getLoggingResponse)
414+
} else if getLoggingResponse.LoggingEnabled == nil && ko.Spec.Logging != nil {
415+
ko.Spec.Logging = &svcapitypes.BucketLoggingStatus{}
416+
}
398417

399418
listMetricsResponse, err := rm.sdkapi.ListBucketMetricsConfigurationsWithContext(ctx, rm.newListBucketMetricsPayload(r))
400419
if err != nil {
401420
return err
402-
}
403-
ko.Spec.Metrics = make([]*svcapitypes.MetricsConfiguration, len(listMetricsResponse.MetricsConfigurationList))
404-
for i, metricsConfiguration := range listMetricsResponse.MetricsConfigurationList {
405-
ko.Spec.Metrics[i] = rm.setResourceMetricsConfiguration(r, metricsConfiguration)
421+
}
422+
if len(listMetricsResponse.MetricsConfigurationList) > 0 {
423+
ko.Spec.Metrics = make([]*svcapitypes.MetricsConfiguration, len(listMetricsResponse.MetricsConfigurationList))
424+
for i, metricsConfiguration := range listMetricsResponse.MetricsConfigurationList {
425+
ko.Spec.Metrics[i] = rm.setResourceMetricsConfiguration(r, metricsConfiguration)
426+
}
427+
} else if len(listMetricsResponse.MetricsConfigurationList) == 0 && len(ko.Spec.Metrics) > 0 {
428+
ko.Spec.Metrics = []*svcapitypes.MetricsConfiguration{}
406429
}
407430

408431
getNotificationResponse, err := rm.sdkapi.GetBucketNotificationConfigurationWithContext(ctx, rm.newGetBucketNotificationPayload(r))
409432
if err != nil {
410433
return err
411434
}
412-
ko.Spec.Notification = rm.setResourceNotification(r, getNotificationResponse)
435+
if getNotificationResponse.LambdaFunctionConfigurations != nil ||
436+
getNotificationResponse.QueueConfigurations != nil ||
437+
getNotificationResponse.TopicConfigurations != nil {
438+
439+
ko.Spec.Notification = rm.setResourceNotification(r, getNotificationResponse)
440+
} else if (getNotificationResponse.LambdaFunctionConfigurations == nil ||
441+
getNotificationResponse.QueueConfigurations == nil ||
442+
getNotificationResponse.TopicConfigurations == nil) && ko.Spec.Notification != nil {
443+
ko.Spec.Notification = &svcapitypes.NotificationConfiguration{}
444+
}
413445

414446
getOwnershipControlsResponse, err := rm.sdkapi.GetBucketOwnershipControlsWithContext(ctx, rm.newGetBucketOwnershipControlsPayload(r))
415447
if err != nil {
416-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "OwnershipControlsNotFoundError" {
417-
getOwnershipControlsResponse = &svcsdk.GetBucketOwnershipControlsOutput{
418-
OwnershipControls: &svcsdk.OwnershipControls{},
419-
}
420-
} else {
448+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "OwnershipControlsNotFoundError" {
421449
return err
422450
}
423451
}
424452
if getOwnershipControlsResponse.OwnershipControls != nil {
425453
ko.Spec.OwnershipControls = rm.setResourceOwnershipControls(r, getOwnershipControlsResponse)
426-
} else {
427-
ko.Spec.OwnershipControls = nil
454+
} else if getOwnershipControlsResponse.OwnershipControls == nil && ko.Spec.OwnershipControls != nil {
455+
ko.Spec.OwnershipControls = &svcapitypes.OwnershipControls{}
428456
}
429457

430458
getPolicyResponse, err := rm.sdkapi.GetBucketPolicyWithContext(ctx, rm.newGetBucketPolicyPayload(r))
431459
if err != nil {
432-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchBucketPolicy" {
433-
getPolicyResponse = &svcsdk.GetBucketPolicyOutput{}
434-
} else {
460+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchBucketPolicy" {
435461
return err
436462
}
437463
}
438-
ko.Spec.Policy = getPolicyResponse.Policy
464+
if getPolicyResponse.Policy != nil {
465+
ko.Spec.Policy = getPolicyResponse.Policy
466+
} else if getPolicyResponse.Policy == nil && ko.Spec.Policy != nil {
467+
ko.Spec.Policy = (&svcsdk.GetBucketPolicyOutput{}).Policy
468+
}
439469

440470
getPublicAccessBlockResponse, err := rm.sdkapi.GetPublicAccessBlockWithContext(ctx, rm.newGetPublicAccessBlockPayload(r))
441471
if err != nil {
442-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchPublicAccessBlockConfiguration" {
443-
getPublicAccessBlockResponse = &svcsdk.GetPublicAccessBlockOutput{}
444-
} else {
472+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchPublicAccessBlockConfiguration" {
445473
return err
446474
}
447475
}
448476
if getPublicAccessBlockResponse.PublicAccessBlockConfiguration != nil {
449477
ko.Spec.PublicAccessBlock = rm.setResourcePublicAccessBlock(r, getPublicAccessBlockResponse)
450-
} else {
451-
ko.Spec.PublicAccessBlock = nil
478+
} else if getPublicAccessBlockResponse.PublicAccessBlockConfiguration == nil && ko.Spec.PublicAccessBlock != nil {
479+
ko.Spec.PublicAccessBlock = &svcapitypes.PublicAccessBlockConfiguration{}
452480
}
453481

454482
getReplicationResponse, err := rm.sdkapi.GetBucketReplicationWithContext(ctx, rm.newGetBucketReplicationPayload(r))
455483
if err != nil {
456-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "ReplicationConfigurationNotFoundError" {
457-
getReplicationResponse = &svcsdk.GetBucketReplicationOutput{}
458-
} else {
484+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "ReplicationConfigurationNotFoundError" {
459485
return err
460486
}
461487
}
462488
if getReplicationResponse.ReplicationConfiguration != nil {
463489
ko.Spec.Replication = rm.setResourceReplication(r, getReplicationResponse)
464-
} else {
465-
ko.Spec.Replication = nil
490+
} else if getReplicationResponse.ReplicationConfiguration == nil && ko.Spec.Replication != nil {
491+
ko.Spec.Replication = &svcapitypes.ReplicationConfiguration{}
466492
}
467493

468494
getRequestPaymentResponse, err := rm.sdkapi.GetBucketRequestPaymentWithContext(ctx, rm.newGetBucketRequestPaymentPayload(r))
469495
if err != nil {
470496
return nil
497+
}
498+
if getRequestPaymentResponse.Payer != nil {
499+
ko.Spec.RequestPayment = rm.setResourceRequestPayment(r, getRequestPaymentResponse)
500+
} else if getRequestPaymentResponse.Payer == nil && ko.Spec.RequestPayment != nil {
501+
ko.Spec.RequestPayment = &svcapitypes.RequestPaymentConfiguration{}
471502
}
472-
ko.Spec.RequestPayment = rm.setResourceRequestPayment(r, getRequestPaymentResponse)
473503

474504
getTaggingResponse, err := rm.sdkapi.GetBucketTaggingWithContext(ctx, rm.newGetBucketTaggingPayload(r))
475505
if err != nil {
476-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchTagSet" {
477-
getTaggingResponse = &svcsdk.GetBucketTaggingOutput{}
478-
} else {
506+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchTagSet" {
479507
return err
480508
}
509+
}
510+
if getTaggingResponse.TagSet != nil {
511+
ko.Spec.Tagging = rm.setResourceTagging(r, getTaggingResponse)
512+
} else if getTaggingResponse.TagSet == nil && ko.Spec.Tagging != nil {
513+
ko.Spec.Tagging = &svcapitypes.Tagging{}
481514
}
482-
ko.Spec.Tagging = rm.setResourceTagging(r, getTaggingResponse)
483515

484516
getVersioningResponse, err := rm.sdkapi.GetBucketVersioningWithContext(ctx, rm.newGetBucketVersioningPayload(r))
485517
if err != nil {
486518
return err
519+
}
520+
if getVersioningResponse.Status != nil {
521+
ko.Spec.Versioning = rm.setResourceVersioning(r, getVersioningResponse)
522+
} else if getVersioningResponse.Status == nil && ko.Spec.Versioning != nil {
523+
ko.Spec.Versioning = &svcapitypes.VersioningConfiguration{}
487524
}
488-
ko.Spec.Versioning = rm.setResourceVersioning(r, getVersioningResponse)
489525

490526
getWebsiteResponse, err := rm.sdkapi.GetBucketWebsiteWithContext(ctx, rm.newGetBucketWebsitePayload(r))
491527
if err != nil {
492-
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "NoSuchWebsiteConfiguration" {
493-
getWebsiteResponse = &svcsdk.GetBucketWebsiteOutput{}
494-
} else {
528+
if awsErr, ok := ackerr.AWSError(err); !ok || awsErr.Code() != "NoSuchWebsiteConfiguration" {
495529
return err
496530
}
531+
}
532+
if getWebsiteResponse != nil {
533+
ko.Spec.Website = rm.setResourceWebsite(r, getWebsiteResponse)
534+
}else if getWebsiteResponse == nil && ko.Spec.Website != nil {
535+
ko.Spec.Website = &svcapitypes.WebsiteConfiguration{}
497536
}
498-
ko.Spec.Website = rm.setResourceWebsite(r, getWebsiteResponse)
537+
499538
return nil
500539
}
501540

@@ -688,16 +727,28 @@ func (rm *resourceManager) setResourceACL(
688727
resp *svcsdk.GetBucketAclOutput,
689728
) {
690729
grants := GetHeadersFromGrants(resp)
691-
ko.Spec.GrantFullControl = &grants.FullControl
692-
ko.Spec.GrantRead = &grants.Read
693-
ko.Spec.GrantReadACP = &grants.ReadACP
694-
ko.Spec.GrantWrite = &grants.Write
695-
ko.Spec.GrantWriteACP = &grants.WriteACP
730+
if grants.FullControl != "" {
731+
ko.Spec.GrantFullControl = &grants.FullControl
732+
}
733+
if grants.Read != "" {
734+
ko.Spec.GrantRead = &grants.Read
735+
}
736+
if grants.ReadACP != "" {
737+
ko.Spec.GrantReadACP = &grants.ReadACP
738+
}
739+
if grants.Write != "" {
740+
ko.Spec.GrantWrite = &grants.Write
741+
}
742+
if grants.WriteACP != "" {
743+
ko.Spec.GrantWriteACP = &grants.WriteACP
744+
}
696745

697746
// Join possible ACLs into a single string, delimited by bar
698747
cannedACLs := GetPossibleCannedACLsFromGrants(resp)
699748
joinedACLs := strings.Join(cannedACLs, CannedACLJoinDelimiter)
700-
ko.Spec.ACL = &joinedACLs
749+
if joinedACLs != "" {
750+
ko.Spec.ACL = &joinedACLs
751+
}
701752
}
702753

703754
func (rm *resourceManager) newGetBucketACLPayload(

0 commit comments

Comments
 (0)