Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

identity: Make UnmarshalK8sServiceAccount a method on SDSCert and deprecate as a standalone function #3220

Closed
1 task
draychev opened this issue Apr 22, 2021 · 0 comments · Fixed by #3387
Closed
1 task
Assignees
Milestone

Comments

@draychev
Copy link
Contributor

draychev commented Apr 22, 2021

The goal of this GitHub issue is to remove complexity and potential errors.

The function identity.UnmarshalK8sServiceAccount()

func UnmarshalK8sServiceAccount(svcAccount string) (*K8sServiceAccount, error) {

is used only on SDSCert.Name here:
svcAccountInRequest, err := identity.UnmarshalK8sServiceAccount(sdscert.Name)

We may have wanted to use this function for many things in the past, but now it is only parsing SDSCert.Name

I propose we remove UnmarshalK8sServiceAccount() as a standalone function and make it a method on SDSCert.

So instead of UnmarshalK8sServiceAccount(sdsCert) we will have sdsCert.GetK8SServiceAccount() (This obviously assumes a rename from UnmarshalK8sServiceAccount to GetK8SServiceAccount).


I propose:

  1. To avoid cyclical imports - create a new package - perhaps pkg/envoy/secrets or something similar
  2. Move SDSCert{} there:

    osm/pkg/envoy/xdsutil.go

    Lines 22 to 37 in 06e70ae

    // SDSCertType is a type of a certificate requested by an Envoy proxy via SDS.
    type SDSCertType string
    // SDSCert is only used to interface the naming and related functions to Marshal/Unmarshal a resource name,
    // this avoids having sprintf/parsing logic all over the place
    type SDSCert struct {
    // Name is the name of the SDS secret for the certificate
    Name string
    // CertType is the certificate type
    CertType SDSCertType
    }
    func (ct SDSCertType) String() string {
    return string(ct)
    }
  3. Make SDSCert.Name be a new type SDSCertName string
  4. Implement SDSCert{}.GetK8SServiceAccount() -- same as UnmarshalK8sServiceAccount(certName)
  5. Move (copy code) tests for UnmarshalK8sServiceAccount for SDSCert{}.GetK8SServiceAccount()
  6. Remove UnmarshalK8sServiceAccount

References

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

Successfully merging a pull request may close this issue.

2 participants