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

fix: address some golangci-lint issues #751

Merged
merged 2 commits into from
Mar 3, 2022

Conversation

hezhizhen
Copy link
Contributor

Description of the change

Benefits

Possible drawbacks

Applicable issues

  • fixes #

Additional information

Copy link
Collaborator

@juan131 juan131 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for this awesome PR removing tech debt! Please check my comments when you have a chance.

cmd/controller/controller.go Outdated Show resolved Hide resolved
Comment on lines -352 to -381
func (c *Controller) updateSecret(ctx context.Context, newSecret *corev1.Secret) (*corev1.Secret, error) {
existingSecret, err := c.sclient.Secrets(newSecret.GetObjectMeta().GetNamespace()).Get(ctx, newSecret.GetObjectMeta().GetName(), metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("failed to read existing secret: %s", err)
}
existingSecret = existingSecret.DeepCopy()
existingSecret.Data = newSecret.Data

c.updateOwnerReferences(existingSecret, newSecret)

return existingSecret, nil
}

func (c *Controller) updateOwnerReferences(existing, new *corev1.Secret) {
ownerRefs := existing.GetOwnerReferences()

for _, newRef := range new.GetOwnerReferences() {
found := false
for _, ref := range ownerRefs {
if newRef.UID == ref.UID {
found = true
break
}
}
if !found {
ownerRefs = append(ownerRefs, newRef)
}
}
existing.SetOwnerReferences(ownerRefs)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkmik do you know why is this unused? Tech debt?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a leftover from some refactoring, possibly when the k8s client API got updated.

it's dead code, let's remove it

cmd/controller/controller.go Show resolved Hide resolved
@juan131 juan131 added backlog Issues/PRs that will be included in the project roadmap enhancement labels Feb 24, 2022
@juan131 juan131 added this to Inbox in Sealed Secrets via automation Feb 24, 2022
@juan131 juan131 moved this from Inbox to In progress in Sealed Secrets Feb 24, 2022
Copy link
Collaborator

@juan131 juan131 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@alvneiayu alvneiayu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alvneiayu alvneiayu merged commit f6dc54e into bitnami-labs:main Mar 3, 2022
Sealed Secrets automation moved this from In progress to Completed Mar 3, 2022
@alvneiayu alvneiayu mentioned this pull request Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues/PRs that will be included in the project roadmap enhancement
Projects
Sealed Secrets
  
Completed
Development

Successfully merging this pull request may close these issues.

None yet

4 participants