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

chore: refactor controller #185

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

chore: refactor controller #185

wants to merge 20 commits into from

Conversation

jodevsa
Copy link
Owner

@jodevsa jodevsa commented Jul 3, 2024

first steps to fix #171

@jodevsa
Copy link
Owner Author

jodevsa commented Jul 3, 2024

@jodevsa
Copy link
Owner Author

jodevsa commented Jul 3, 2024

@jodevsa
Copy link
Owner Author

jodevsa commented Jul 4, 2024

the operator code is so much coupled. Part of it is kind-of related to operator-sdk not providing any framework or abstractions for building operator resources. I'll be creating some abstractions in a separate repo to act as a base for operator builders. This is a lot of work but is something that is crucial and needed for the community.

@jodevsa
Copy link
Owner Author

jodevsa commented Jul 5, 2024

running them will result in the following changes:

Screenshot 2024-07-05 at 15 00 23

return false, nil
}
func (p Peers) getPeerConfig(peer v1alpha1.WireguardPeer) string {
cfg := fmt.Sprintf(`
Copy link
Collaborator

@uhthomas uhthomas Jul 7, 2024

Choose a reason for hiding this comment

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

I wonder if we should actually encode real object instead of string formatting. We wouldn't do this for yaml or json, so why from toml?

Hypothetically:

type WireguardConfiguration struct {
    Interface WireguardInterfaceConfiguration
    // ...
}

type WireguardInterfaceConfiguration struct {
    PrivateKey string
    Address string
    DNS string
}

b, err := toml.Marshal(WireguardConfiguration{
    Interface: WireguardInterfaceConfiguration{
        PrivateKey: fmt.Sprintf("$(kubectl get secret %s-peer --template={{.data.privateKey}} -n %s | base64 -d)", peer.Name, peer.Namespace),
        Address: peer.Spec.Address
        DNS: p.constructDnsConfig()
    },
})
// ...

Comment on lines +61 to +66
if !bytes.Equal(expectedSecret.Data["state.json"], secret.Data["state.json"]) {
return true, nil

}

return false, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if !bytes.Equal(expectedSecret.Data["state.json"], secret.Data["state.json"]) {
return true, nil
}
return false, nil
return !bytes.Equal(expectedSecret.Data["state.json"], secret.Data["state.json"]), nil

Comment on lines +143 to +145
} else if err == nil {
privateKey = string(sec.Data["privateKey"])
publicKey = string(sec.Data["publicKey"])
Copy link
Collaborator

@uhthomas uhthomas Jul 7, 2024

Choose a reason for hiding this comment

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

This should be moved, as everything after the if err != nil line matches this condition

@@ -0,0 +1,176 @@
package resources

import (
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs formatting (gofmt -s -w .)

}

func (s Service) GetAddressAndPort(ctx context.Context) (string, string, error) {
var port = fmt.Sprintf("%d", s.TargetPort)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
var port = fmt.Sprintf("%d", s.TargetPort)
port := strconv.Itoa(s.TargetPort)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential for resource name conflicts
2 participants