Skip to content

Commit 939ad43

Browse files
michaelhtmmichaelhtm
authored
fix: omitUnchangedFields except for status fields and required fields (aws-controllers-k8s#654)
Description of changes: Currently omitUnchanged fields was applied to all spec fields. This raises issues when certain required fields for update operations are in the spec. With this fix, we make sure to not omit required fields even if they are in spec By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent ca61a72 commit 939ad43

3 files changed

Lines changed: 313 additions & 2 deletions

File tree

‎pkg/generate/code/set_sdk.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func SetSDK(
322322
// }
323323

324324
omitUnchangedFieldsOnUpdate := op == r.Ops.Update && r.OmitUnchangedFieldsOnUpdate()
325-
if omitUnchangedFieldsOnUpdate && inSpec {
325+
if omitUnchangedFieldsOnUpdate && inSpec && !inputShape.IsRequired(memberName) {
326326
fieldJSONPath := fmt.Sprintf("%s.%s", cfg.PrefixConfig.SpecField[1:], f.Names.Camel)
327327
out += fmt.Sprintf(
328328
"%sif delta.DifferentAt(%q) {\n", indent, fieldJSONPath,
@@ -408,7 +408,7 @@ func SetSDK(
408408
"%s}\n", indent,
409409
)
410410

411-
if omitUnchangedFieldsOnUpdate && inSpec {
411+
if omitUnchangedFieldsOnUpdate && inSpec && !inputShape.IsRequired(memberName) {
412412
// decrease indentation level
413413
indentLevel--
414414
indent = indent[1:]

‎pkg/generate/code/set_sdk_test.go‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,6 +1636,315 @@ func TestSetSDK_RDS_DBInstance_Create(t *testing.T) {
16361636
)
16371637
}
16381638

1639+
func TestSetSDK_RDS_DBInstance_Update(t *testing.T) {
1640+
assert := assert.New(t)
1641+
require := require.New(t)
1642+
1643+
g := testutil.NewModelForService(t, "rds")
1644+
1645+
crd := testutil.GetCRDByName(t, g, "DBInstance")
1646+
require.NotNil(crd)
1647+
1648+
expected := `
1649+
if delta.DifferentAt("Spec.AllocatedStorage") {
1650+
if r.ko.Spec.AllocatedStorage != nil {
1651+
allocatedStorageCopy0 := *r.ko.Spec.AllocatedStorage
1652+
if allocatedStorageCopy0 > math.MaxInt32 || allocatedStorageCopy0 < math.MinInt32 {
1653+
return nil, fmt.Errorf("error: field AllocatedStorage is of type int32")
1654+
}
1655+
allocatedStorageCopy := int32(allocatedStorageCopy0)
1656+
res.AllocatedStorage = &allocatedStorageCopy
1657+
}
1658+
}
1659+
res.AllowMajorVersionUpgrade = aws.Bool(true)
1660+
res.ApplyImmediately = aws.Bool(true)
1661+
if delta.DifferentAt("Spec.AutoMinorVersionUpgrade") {
1662+
if r.ko.Spec.AutoMinorVersionUpgrade != nil {
1663+
res.AutoMinorVersionUpgrade = r.ko.Spec.AutoMinorVersionUpgrade
1664+
}
1665+
}
1666+
if r.ko.Status.AutomationMode != nil {
1667+
res.AutomationMode = svcsdktypes.AutomationMode(*r.ko.Status.AutomationMode)
1668+
}
1669+
if r.ko.Status.AWSBackupRecoveryPointARN != nil {
1670+
res.AwsBackupRecoveryPointArn = r.ko.Status.AWSBackupRecoveryPointARN
1671+
}
1672+
if delta.DifferentAt("Spec.BackupRetentionPeriod") {
1673+
if r.ko.Spec.BackupRetentionPeriod != nil {
1674+
backupRetentionPeriodCopy0 := *r.ko.Spec.BackupRetentionPeriod
1675+
if backupRetentionPeriodCopy0 > math.MaxInt32 || backupRetentionPeriodCopy0 < math.MinInt32 {
1676+
return nil, fmt.Errorf("error: field BackupRetentionPeriod is of type int32")
1677+
}
1678+
backupRetentionPeriodCopy := int32(backupRetentionPeriodCopy0)
1679+
res.BackupRetentionPeriod = &backupRetentionPeriodCopy
1680+
}
1681+
}
1682+
if delta.DifferentAt("Spec.CACertificateIdentifier") {
1683+
if r.ko.Spec.CACertificateIdentifier != nil {
1684+
res.CACertificateIdentifier = r.ko.Spec.CACertificateIdentifier
1685+
}
1686+
}
1687+
if delta.DifferentAt("Spec.CopyTagsToSnapshot") {
1688+
if r.ko.Spec.CopyTagsToSnapshot != nil {
1689+
res.CopyTagsToSnapshot = r.ko.Spec.CopyTagsToSnapshot
1690+
}
1691+
}
1692+
if delta.DifferentAt("Spec.DBInstanceClass") {
1693+
if r.ko.Spec.DBInstanceClass != nil {
1694+
res.DBInstanceClass = r.ko.Spec.DBInstanceClass
1695+
}
1696+
}
1697+
if r.ko.Spec.DBInstanceIdentifier != nil {
1698+
res.DBInstanceIdentifier = r.ko.Spec.DBInstanceIdentifier
1699+
}
1700+
if delta.DifferentAt("Spec.DBParameterGroupName") {
1701+
if r.ko.Spec.DBParameterGroupName != nil {
1702+
res.DBParameterGroupName = r.ko.Spec.DBParameterGroupName
1703+
}
1704+
}
1705+
if delta.DifferentAt("Spec.DBSubnetGroupName") {
1706+
if r.ko.Spec.DBSubnetGroupName != nil {
1707+
res.DBSubnetGroupName = r.ko.Spec.DBSubnetGroupName
1708+
}
1709+
}
1710+
if r.ko.Status.DatabaseInsightsMode != nil {
1711+
res.DatabaseInsightsMode = svcsdktypes.DatabaseInsightsMode(*r.ko.Status.DatabaseInsightsMode)
1712+
}
1713+
if r.ko.Status.DedicatedLogVolume != nil {
1714+
res.DedicatedLogVolume = r.ko.Status.DedicatedLogVolume
1715+
}
1716+
if delta.DifferentAt("Spec.DeletionProtection") {
1717+
if r.ko.Spec.DeletionProtection != nil {
1718+
res.DeletionProtection = r.ko.Spec.DeletionProtection
1719+
}
1720+
}
1721+
if delta.DifferentAt("Spec.Domain") {
1722+
if r.ko.Spec.Domain != nil {
1723+
res.Domain = r.ko.Spec.Domain
1724+
}
1725+
}
1726+
if delta.DifferentAt("Spec.DomainFqdn") {
1727+
if r.ko.Spec.DomainFqdn != nil {
1728+
res.DomainFqdn = r.ko.Spec.DomainFqdn
1729+
}
1730+
}
1731+
if delta.DifferentAt("Spec.DomainIAMRoleName") {
1732+
if r.ko.Spec.DomainIAMRoleName != nil {
1733+
res.DomainIAMRoleName = r.ko.Spec.DomainIAMRoleName
1734+
}
1735+
}
1736+
if delta.DifferentAt("Spec.DomainOu") {
1737+
if r.ko.Spec.DomainOu != nil {
1738+
res.DomainOu = r.ko.Spec.DomainOu
1739+
}
1740+
}
1741+
if delta.DifferentAt("Spec.EnableCustomerOwnedIP") {
1742+
if r.ko.Spec.EnableCustomerOwnedIP != nil {
1743+
res.EnableCustomerOwnedIp = r.ko.Spec.EnableCustomerOwnedIP
1744+
}
1745+
}
1746+
if delta.DifferentAt("Spec.EnableIAMDatabaseAuthentication") {
1747+
if r.ko.Spec.EnableIAMDatabaseAuthentication != nil {
1748+
res.EnableIAMDatabaseAuthentication = r.ko.Spec.EnableIAMDatabaseAuthentication
1749+
}
1750+
}
1751+
if delta.DifferentAt("Spec.PerformanceInsightsEnabled") {
1752+
if r.ko.Spec.PerformanceInsightsEnabled != nil {
1753+
res.EnablePerformanceInsights = r.ko.Spec.PerformanceInsightsEnabled
1754+
}
1755+
}
1756+
if delta.DifferentAt("Spec.Engine") {
1757+
if r.ko.Spec.Engine != nil {
1758+
res.Engine = r.ko.Spec.Engine
1759+
}
1760+
}
1761+
if delta.DifferentAt("Spec.EngineVersion") {
1762+
if r.ko.Spec.EngineVersion != nil {
1763+
res.EngineVersion = r.ko.Spec.EngineVersion
1764+
}
1765+
}
1766+
if delta.DifferentAt("Spec.IOPS") {
1767+
if r.ko.Spec.IOPS != nil {
1768+
iopsCopy0 := *r.ko.Spec.IOPS
1769+
if iopsCopy0 > math.MaxInt32 || iopsCopy0 < math.MinInt32 {
1770+
return nil, fmt.Errorf("error: field Iops is of type int32")
1771+
}
1772+
iopsCopy := int32(iopsCopy0)
1773+
res.Iops = &iopsCopy
1774+
}
1775+
}
1776+
if delta.DifferentAt("Spec.LicenseModel") {
1777+
if r.ko.Spec.LicenseModel != nil {
1778+
res.LicenseModel = r.ko.Spec.LicenseModel
1779+
}
1780+
}
1781+
if delta.DifferentAt("Spec.ManageMasterUserPassword") {
1782+
if r.ko.Spec.ManageMasterUserPassword != nil {
1783+
res.ManageMasterUserPassword = r.ko.Spec.ManageMasterUserPassword
1784+
}
1785+
}
1786+
if delta.DifferentAt("Spec.MasterUserPassword") {
1787+
if r.ko.Spec.MasterUserPassword != nil {
1788+
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.MasterUserPassword)
1789+
if err != nil {
1790+
return nil, ackrequeue.Needed(err)
1791+
}
1792+
if tmpSecret != "" {
1793+
res.MasterUserPassword = aws.String(tmpSecret)
1794+
}
1795+
}
1796+
}
1797+
if delta.DifferentAt("Spec.MasterUserSecretKMSKeyID") {
1798+
if r.ko.Spec.MasterUserSecretKMSKeyID != nil {
1799+
res.MasterUserSecretKmsKeyId = r.ko.Spec.MasterUserSecretKMSKeyID
1800+
}
1801+
}
1802+
if delta.DifferentAt("Spec.MaxAllocatedStorage") {
1803+
if r.ko.Spec.MaxAllocatedStorage != nil {
1804+
maxAllocatedStorageCopy0 := *r.ko.Spec.MaxAllocatedStorage
1805+
if maxAllocatedStorageCopy0 > math.MaxInt32 || maxAllocatedStorageCopy0 < math.MinInt32 {
1806+
return nil, fmt.Errorf("error: field MaxAllocatedStorage is of type int32")
1807+
}
1808+
maxAllocatedStorageCopy := int32(maxAllocatedStorageCopy0)
1809+
res.MaxAllocatedStorage = &maxAllocatedStorageCopy
1810+
}
1811+
}
1812+
if delta.DifferentAt("Spec.MonitoringInterval") {
1813+
if r.ko.Spec.MonitoringInterval != nil {
1814+
monitoringIntervalCopy0 := *r.ko.Spec.MonitoringInterval
1815+
if monitoringIntervalCopy0 > math.MaxInt32 || monitoringIntervalCopy0 < math.MinInt32 {
1816+
return nil, fmt.Errorf("error: field MonitoringInterval is of type int32")
1817+
}
1818+
monitoringIntervalCopy := int32(monitoringIntervalCopy0)
1819+
res.MonitoringInterval = &monitoringIntervalCopy
1820+
}
1821+
}
1822+
if delta.DifferentAt("Spec.MonitoringRoleARN") {
1823+
if r.ko.Spec.MonitoringRoleARN != nil {
1824+
res.MonitoringRoleArn = r.ko.Spec.MonitoringRoleARN
1825+
}
1826+
}
1827+
if delta.DifferentAt("Spec.MultiAZ") {
1828+
if r.ko.Spec.MultiAZ != nil {
1829+
res.MultiAZ = r.ko.Spec.MultiAZ
1830+
}
1831+
}
1832+
if r.ko.Status.MultiTenant != nil {
1833+
res.MultiTenant = r.ko.Status.MultiTenant
1834+
}
1835+
if delta.DifferentAt("Spec.NetworkType") {
1836+
if r.ko.Spec.NetworkType != nil {
1837+
res.NetworkType = r.ko.Spec.NetworkType
1838+
}
1839+
}
1840+
if delta.DifferentAt("Spec.OptionGroupName") {
1841+
if r.ko.Spec.OptionGroupName != nil {
1842+
res.OptionGroupName = r.ko.Spec.OptionGroupName
1843+
}
1844+
}
1845+
if delta.DifferentAt("Spec.PerformanceInsightsKMSKeyID") {
1846+
if r.ko.Spec.PerformanceInsightsKMSKeyID != nil {
1847+
res.PerformanceInsightsKMSKeyId = r.ko.Spec.PerformanceInsightsKMSKeyID
1848+
}
1849+
}
1850+
if delta.DifferentAt("Spec.PerformanceInsightsRetentionPeriod") {
1851+
if r.ko.Spec.PerformanceInsightsRetentionPeriod != nil {
1852+
performanceInsightsRetentionPeriodCopy0 := *r.ko.Spec.PerformanceInsightsRetentionPeriod
1853+
if performanceInsightsRetentionPeriodCopy0 > math.MaxInt32 || performanceInsightsRetentionPeriodCopy0 < math.MinInt32 {
1854+
return nil, fmt.Errorf("error: field PerformanceInsightsRetentionPeriod is of type int32")
1855+
}
1856+
performanceInsightsRetentionPeriodCopy := int32(performanceInsightsRetentionPeriodCopy0)
1857+
res.PerformanceInsightsRetentionPeriod = &performanceInsightsRetentionPeriodCopy
1858+
}
1859+
}
1860+
if delta.DifferentAt("Spec.PreferredBackupWindow") {
1861+
if r.ko.Spec.PreferredBackupWindow != nil {
1862+
res.PreferredBackupWindow = r.ko.Spec.PreferredBackupWindow
1863+
}
1864+
}
1865+
if delta.DifferentAt("Spec.PreferredMaintenanceWindow") {
1866+
if r.ko.Spec.PreferredMaintenanceWindow != nil {
1867+
res.PreferredMaintenanceWindow = r.ko.Spec.PreferredMaintenanceWindow
1868+
}
1869+
}
1870+
if delta.DifferentAt("Spec.ProcessorFeatures") {
1871+
if r.ko.Spec.ProcessorFeatures != nil {
1872+
f49 := []svcsdktypes.ProcessorFeature{}
1873+
for _, f49iter := range r.ko.Spec.ProcessorFeatures {
1874+
f49elem := &svcsdktypes.ProcessorFeature{}
1875+
if f49iter.Name != nil {
1876+
f49elem.Name = f49iter.Name
1877+
}
1878+
if f49iter.Value != nil {
1879+
f49elem.Value = f49iter.Value
1880+
}
1881+
f49 = append(f49, *f49elem)
1882+
}
1883+
res.ProcessorFeatures = f49
1884+
}
1885+
}
1886+
if delta.DifferentAt("Spec.PromotionTier") {
1887+
if r.ko.Spec.PromotionTier != nil {
1888+
promotionTierCopy0 := *r.ko.Spec.PromotionTier
1889+
if promotionTierCopy0 > math.MaxInt32 || promotionTierCopy0 < math.MinInt32 {
1890+
return nil, fmt.Errorf("error: field PromotionTier is of type int32")
1891+
}
1892+
promotionTierCopy := int32(promotionTierCopy0)
1893+
res.PromotionTier = &promotionTierCopy
1894+
}
1895+
}
1896+
if delta.DifferentAt("Spec.PubliclyAccessible") {
1897+
if r.ko.Spec.PubliclyAccessible != nil {
1898+
res.PubliclyAccessible = r.ko.Spec.PubliclyAccessible
1899+
}
1900+
}
1901+
if delta.DifferentAt("Spec.ReplicaMode") {
1902+
if r.ko.Spec.ReplicaMode != nil {
1903+
res.ReplicaMode = svcsdktypes.ReplicaMode(*r.ko.Spec.ReplicaMode)
1904+
}
1905+
}
1906+
if delta.DifferentAt("Spec.StorageThroughput") {
1907+
if r.ko.Spec.StorageThroughput != nil {
1908+
storageThroughputCopy0 := *r.ko.Spec.StorageThroughput
1909+
if storageThroughputCopy0 > math.MaxInt32 || storageThroughputCopy0 < math.MinInt32 {
1910+
return nil, fmt.Errorf("error: field StorageThroughput is of type int32")
1911+
}
1912+
storageThroughputCopy := int32(storageThroughputCopy0)
1913+
res.StorageThroughput = &storageThroughputCopy
1914+
}
1915+
}
1916+
if delta.DifferentAt("Spec.StorageType") {
1917+
if r.ko.Spec.StorageType != nil {
1918+
res.StorageType = r.ko.Spec.StorageType
1919+
}
1920+
}
1921+
if delta.DifferentAt("Spec.TDECredentialARN") {
1922+
if r.ko.Spec.TDECredentialARN != nil {
1923+
res.TdeCredentialArn = r.ko.Spec.TDECredentialARN
1924+
}
1925+
}
1926+
if delta.DifferentAt("Spec.TDECredentialPassword") {
1927+
if r.ko.Spec.TDECredentialPassword != nil {
1928+
res.TdeCredentialPassword = r.ko.Spec.TDECredentialPassword
1929+
}
1930+
}
1931+
if delta.DifferentAt("Spec.UseDefaultProcessorFeatures") {
1932+
if r.ko.Spec.UseDefaultProcessorFeatures != nil {
1933+
res.UseDefaultProcessorFeatures = r.ko.Spec.UseDefaultProcessorFeatures
1934+
}
1935+
}
1936+
if delta.DifferentAt("Spec.VPCSecurityGroupIDs") {
1937+
if r.ko.Spec.VPCSecurityGroupIDs != nil {
1938+
res.VpcSecurityGroupIds = aws.ToStringSlice(r.ko.Spec.VPCSecurityGroupIDs)
1939+
}
1940+
}
1941+
`
1942+
assert.Equal(
1943+
expected,
1944+
code.SetSDK(crd.Config(), crd, model.OpTypeUpdate, "r.ko", "res", 1),
1945+
)
1946+
}
1947+
16391948
func TestSetSDK_S3_Bucket_Create(t *testing.T) {
16401949
assert := assert.New(t)
16411950
require := require.New(t)

‎pkg/testdata/models/apis/rds/0000-00-00/generator.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ resources:
205205
path: Parameters
206206
is_read_only: true
207207
DBInstance:
208+
update_operation:
209+
omit_unchanged_fields: true
208210
fields:
209211
AvailabilityZone:
210212
late_initialize: {}

0 commit comments

Comments
 (0)