Skip to content

Commit 9c1e0d1

Browse files
committed
prevent patches from patching out required fields
it was possible to write a patch like `spec: null` that removed fields that the controller expects to exist, even after patching
1 parent 08214b7 commit 9c1e0d1

File tree

2 files changed

+750
-253
lines changed

2 files changed

+750
-253
lines changed

pkg/config/config.go

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -494,10 +494,9 @@ func (c *Config) ServiceAccount() *applycorev1.ServiceAccountApplyConfiguration
494494
_, _, _ = ApplyPatches(c.unpatchedServiceAccount(), sa, c.Patches, c.Resources)
495495

496496
// ensure patches don't overwrite anything critical for operator function
497-
sa.WithName(c.ServiceAccountName).WithNamespace(c.Namespace).
497+
return sa.WithName(c.ServiceAccountName).WithNamespace(c.Namespace).
498498
WithLabels(metadata.LabelsForComponent(c.Name, metadata.ComponentServiceAccountLabel)).
499499
WithOwnerReferences(c.ownerRef())
500-
return sa
501500
}
502501

503502
func (c *Config) unpatchedRole() *applyrbacv1.RoleApplyConfiguration {
@@ -516,10 +515,9 @@ func (c *Config) Role() *applyrbacv1.RoleApplyConfiguration {
516515
_, _, _ = ApplyPatches(c.unpatchedRole(), role, c.Patches, c.Resources)
517516

518517
// ensure patches don't overwrite anything critical for operator function
519-
role.WithName(c.Name).WithNamespace(c.Namespace).
518+
return role.WithName(c.Name).WithNamespace(c.Namespace).
520519
WithLabels(metadata.LabelsForComponent(c.Name, metadata.ComponentRoleLabel)).
521520
WithOwnerReferences(c.ownerRef())
522-
return role
523521
}
524522

525523
func (c *Config) unpatchedRoleBinding() *applyrbacv1.RoleBindingApplyConfiguration {
@@ -539,10 +537,9 @@ func (c *Config) RoleBinding() *applyrbacv1.RoleBindingApplyConfiguration {
539537
_, _, _ = ApplyPatches(c.unpatchedRoleBinding(), rb, c.Patches, c.Resources)
540538

541539
// ensure patches don't overwrite anything critical for operator function
542-
rb.WithName(c.Name).WithNamespace(c.Namespace).
540+
return rb.WithName(c.Name).WithNamespace(c.Namespace).
543541
WithLabels(metadata.LabelsForComponent(c.Name, metadata.ComponentRoleBindingLabel)).
544542
WithOwnerReferences(c.ownerRef())
545-
return rb
546543
}
547544

548545
func (c *Config) unpatchedService() *applycorev1.ServiceApplyConfiguration {
@@ -556,7 +553,13 @@ func (c *Config) unpatchedService() *applycorev1.ServiceApplyConfiguration {
556553

557554
func (c *Config) Service() *applycorev1.ServiceApplyConfiguration {
558555
s := applycorev1.Service(c.Name, c.Namespace)
559-
_, _, _ = ApplyPatches(c.unpatchedService(), s, c.Patches, c.Resources)
556+
unpatched := c.unpatchedService()
557+
_, _, _ = ApplyPatches(unpatched, s, c.Patches, c.Resources)
558+
559+
// not allowed to patch out the spec
560+
if s.Spec == nil {
561+
s.Spec = unpatched.Spec
562+
}
560563

561564
// ensure patches don't overwrite anything critical for operator function
562565
s.WithName(c.Name).WithNamespace(c.Namespace).
@@ -691,7 +694,18 @@ func (c *Config) unpatchedMigrationJob(migrationHash string) *applybatchv1.JobAp
691694

692695
func (c *Config) MigrationJob(migrationHash string) *applybatchv1.JobApplyConfiguration {
693696
j := applybatchv1.Job(c.jobName(migrationHash), c.Namespace)
694-
_, _, _ = ApplyPatches(c.unpatchedMigrationJob(migrationHash), j, c.Patches, c.Resources)
697+
unpatched := c.unpatchedMigrationJob(migrationHash)
698+
_, _, _ = ApplyPatches(unpatched, j, c.Patches, c.Resources)
699+
700+
// not allowed to patch out the spec
701+
if j.Spec == nil {
702+
j.Spec = unpatched.Spec
703+
}
704+
705+
// not allowed to patch out the template
706+
if j.Spec.Template == nil {
707+
j.Spec.Template = unpatched.Spec.Template
708+
}
695709

696710
// ensure patches don't overwrite anything critical for operator function
697711
name := c.jobName(migrationHash)
@@ -801,7 +815,23 @@ func (c *Config) unpatchedDeployment(migrationHash, secretHash string) *applyapp
801815
func (c *Config) Deployment(migrationHash, secretHash string) *applyappsv1.DeploymentApplyConfiguration {
802816
name := deploymentName(c.Name)
803817
d := applyappsv1.Deployment(name, c.Namespace)
804-
_, _, _ = ApplyPatches(c.unpatchedDeployment(migrationHash, secretHash), d, c.Patches, c.Resources)
818+
unpatched := c.unpatchedDeployment(migrationHash, secretHash)
819+
_, _, _ = ApplyPatches(unpatched, d, c.Patches, c.Resources)
820+
821+
// not allowed to patch out the spec
822+
if d.Spec == nil {
823+
d.Spec = unpatched.Spec
824+
}
825+
826+
// not allowed to patch out the selector
827+
if d.Spec.Selector == nil {
828+
d.Spec.Selector = unpatched.Spec.Selector
829+
}
830+
831+
// not allowed to patch out the template
832+
if d.Spec.Template == nil {
833+
d.Spec.Template = unpatched.Spec.Template
834+
}
805835

806836
// ensure patches don't overwrite anything critical for operator function
807837
d.WithName(name).WithNamespace(c.Namespace).WithOwnerReferences(c.ownerRef()).

0 commit comments

Comments
 (0)