Skip to content

Commit

Permalink
Changed yaml unmarshal to strict type for structs
Browse files Browse the repository at this point in the history
  • Loading branch information
abhishekdwivedi3060 committed Aug 22, 2024
1 parent 392abcb commit 6bfe93b
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 14 deletions.
6 changes: 3 additions & 3 deletions api/v1beta1/aerospikebackup_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (r *AerospikeBackup) validateBackupConfig() error {

var backupSvcConfig model.Config

if err := yaml.Unmarshal(backupSvc.Spec.Config.Raw, &backupSvcConfig); err != nil {
if err := yaml.UnmarshalStrict(backupSvc.Spec.Config.Raw, &backupSvcConfig); err != nil {
return err
}

Expand Down Expand Up @@ -218,7 +218,7 @@ func (r *AerospikeBackup) getValidatedAerospikeClusters(backupConfig map[string]

aeroClusters := make(map[string]*model.AerospikeCluster)

if err := yaml.Unmarshal(clusterBytes, &aeroClusters); err != nil {
if err := yaml.UnmarshalStrict(clusterBytes, &aeroClusters); err != nil {
return nil, err
}

Expand Down Expand Up @@ -304,7 +304,7 @@ func (r *AerospikeBackup) getValidatedBackupRoutines(

backupRoutines := make(map[string]*model.BackupRoutine)

if err := yaml.Unmarshal(routineBytes, &backupRoutines); err != nil {
if err := yaml.UnmarshalStrict(routineBytes, &backupRoutines); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/aerospikebackupservice_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (r *AerospikeBackupService) ValidateDelete() (admission.Warnings, error) {
func (r *AerospikeBackupService) validateBackupServiceConfig() error {
var config model.Config

if err := yaml.Unmarshal(r.Spec.Config.Raw, &config); err != nil {
if err := yaml.UnmarshalStrict(r.Spec.Config.Raw, &config); err != nil {
return err
}

Expand Down
14 changes: 7 additions & 7 deletions api/v1beta1/aerospikerestore_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,25 +100,25 @@ func (r *AerospikeRestore) ValidateDelete() (admission.Warnings, error) {
}

func (r *AerospikeRestore) validateRestoreConfig() error {
restoreConfigInMap := make(map[string]interface{})
restoreConfig := make(map[string]interface{})

if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreConfigInMap); err != nil {
if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreConfig); err != nil {
return err
}

switch r.Spec.Type {
case Full, Incremental:
var restoreRequest model.RestoreRequest

if _, ok := restoreConfigInMap[common.RoutineKey]; ok {
if _, ok := restoreConfig[common.RoutineKey]; ok {
return fmt.Errorf("routine field is not allowed in restore config for restore type %s", r.Spec.Type)
}

if _, ok := restoreConfigInMap[common.TimeKey]; ok {
if _, ok := restoreConfig[common.TimeKey]; ok {
return fmt.Errorf("time field is not allowed in restore config for restore type %s", r.Spec.Type)
}

if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreRequest); err != nil {
if err := yaml.UnmarshalStrict(r.Spec.Config.Raw, &restoreRequest); err != nil {
return err
}

Expand All @@ -127,11 +127,11 @@ func (r *AerospikeRestore) validateRestoreConfig() error {
case Timestamp:
var restoreRequest model.RestoreTimestampRequest

if _, ok := restoreConfigInMap[common.SourceKey]; ok {
if _, ok := restoreConfig[common.SourceKey]; ok {
return fmt.Errorf("source field is not allowed in restore config for restore type %s", r.Spec.Type)
}

if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreRequest); err != nil {
if err := yaml.UnmarshalStrict(r.Spec.Config.Raw, &restoreRequest); err != nil {
return err
}

Expand Down
15 changes: 15 additions & 0 deletions test/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ var _ = Describe(
ContainSubstring("name should start with %s", namePrefix(backupNsNm)))
})

It("Should fail when un-supported field is given in backup config", func() {
config := getBackupConfigInMap(namePrefix(backupNsNm))
routines := config[common.BackupRoutinesKey].(map[string]interface{})
routines[namePrefix(backupNsNm)+"-"+"test-routine"].(map[string]interface{})["unknown"] = "unknown"
config[common.BackupRoutinesKey] = routines

configBytes, mErr := json.Marshal(config)
Expect(mErr).ToNot(HaveOccurred())

backup = newBackupWithConfig(backupNsNm, configBytes)
err = CreateBackup(k8sClient, backup)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unknown field"))
})

It("Should fail when more than 1 cluster is given in backup config", func() {
config := getBackupConfigInMap(namePrefix(backupNsNm))
aeroCluster := config[common.AerospikeClusterKey].(map[string]interface{})
Expand Down
16 changes: 15 additions & 1 deletion test/backup_service/backup_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var _ = Describe(

Context(
"When doing Invalid operations", func() {
It("Should fail when wrong format backup config is given", func() {
It("Should fail when wrong format backup service config is given", func() {
badConfig, gErr := getWrongBackupServiceConfBytes()
Expect(gErr).ToNot(HaveOccurred())
backupService = newBackupServiceWithConfig(badConfig)
Expand All @@ -36,6 +36,20 @@ var _ = Describe(
Expect(err).To(HaveOccurred())
})

It("Should fail when un-supported field is given in backup service config", func() {
configMap := getBackupServiceConfMap()
configMap["unknown"] = "unknown"

configBytes, mErr := json.Marshal(configMap)
Expect(mErr).ToNot(HaveOccurred())

backupService = newBackupServiceWithConfig(configBytes)

err = DeployBackupService(k8sClient, backupService)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unknown field"))
})

It("Should fail when wrong image is given", func() {
backupService, err = NewBackupService()
Expect(err).ToNot(HaveOccurred())
Expand Down
2 changes: 0 additions & 2 deletions test/backup_service/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,10 @@ func getBackupServiceConfMap() map[string]interface{} {
"test-policy": map[string]interface{}{
"parallel": 3,
"remove-files": "KeepAll",
"type": 1,
},
"test-policy1": map[string]interface{}{
"parallel": 3,
"remove-files": "KeepAll",
"type": 1,
},
},
common.StorageKey: map[string]interface{}{
Expand Down
13 changes: 13 additions & 0 deletions test/restore/restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ var _ = Describe(
Expect(err).To(HaveOccurred())
})

It("Should fail when un-supported field is given in restore config", func() {
config := getRestoreConfigInMap(backupDataPath)
config["unknown"] = "unknown"

configBytes, mErr := json.Marshal(config)
Expect(mErr).ToNot(HaveOccurred())

restore = newRestoreWithConfig(restoreNsNm, asdbv1beta1.Full, configBytes)
err = createRestore(k8sClient, restore)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unknown field"))
})

It("Should fail when spec is updated", func() {
restore, err = newRestore(restoreNsNm, asdbv1beta1.Full)
Expect(err).ToNot(HaveOccurred())
Expand Down

0 comments on commit 6bfe93b

Please sign in to comment.