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

Refactor to providers #270

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Refactor to providers #270

merged 2 commits into from
Jan 29, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Jan 28, 2021

GslbAssistant
Screenshot 2021-01-28 at 15 04 23

IDNSProvider

  • implementation Route53, NS1
  • having two functions (HandleZoneDelegation, Finalize)
  • implement ExternalDNSProvider
  • GetGSLBIngressIPs -> GslbIngressExposedIPs
  • EnsureDNSEndpoint -> SaveDNSEndpoint
  • RemoveEndpoint

Create private functions in providers/dns package

  • nsServerName
  • nsServerNameExt
  • getExternalClusterHeartbeatFQDNs
  • cover all of them by tests

GslbAssistant

  • implementis IGslbAssistant interface
  • common functionality for providers mostly exposes endpoints CRUD
  • accepts Logger interface and could log
  • GetGSLBIngressIPs -> GslbIngressExposedIPs
  • EnsureDNSEndpoint -> SaveDNSEndpoint
  • add RemoveEndpoint
  • checkTXTRecord moved to assistant, fixed tests; renamed to InspectTXTThreshold
  • remove gslb parameter from assistant.RemoveEndpoint
  • remove error output argument from GetExternalTargets
  • overrideWithFakeDNS moved to assistant as private func
  • accepts Logger interface and could log

IDNSProvider

  • implementation Route53, NS1
  • having two functions (HandleZoneDelegation, Finalize)
  • implement ExternalDNSProvider
  • DNSProvider.SaveDNSEndpoint instead of ensureDNSEndpoint

ExternalDNS:

  • move CreateZoneDelegationForExternalDNS to ExternalDNS
  • fixed, missing finalizer for NS1 (Missing finalizer for NS1 #262)
  • fix tests, If config is modified, new provider needs to be re-created. There is no way and reason to change provider
    configuration at another time than startup

Infoblox:

  • split infoblox logic into three package files
  • integrate Infoblox, audit logger fix, no log.Info(fmt.Srintf(...)) needed now, audit.Info(..) takes care..
  • InfobloxDNSProvider contains InfobloxConnection; fakeInfobloxConnection

EmptyDNS

  • Finalize runs this: p.assistant.RemoveEndpoint(gslb.Name)
  • CreateZoneDelegationForExternalDNS returns nil;
  • The rest is implemented the very same way as ExternalDNSPRovider

DNS package tests

  • TestCanGenerateExternalHeartbeatFQDNs moved to dns package
  • TestCanFilterOutDelegatedZoneEntryAccordingFQDNProvided moved from controller-tests to infoblox
  • TestGeneratesProperExternalNSTargetFQDNsAccordingToTheGeoTags moved to common_test but no completely refactored

Other:

  • deprecated NewGone() to NewResourceExpired()
  • implement Stringer interface by providers; now fmt.Sprintf(%s,providerInstance) is writes meaningful message

Factory implementation
Factory tests
Integrate factory with the rest of the code

  • changes in main.go, instantiate IDNSProvider
  • dnsupdate.go, remove CreateZoneDelegationForExternalDNS
  • controller, using r.DNSProvider.CreateZoneDelegationForExternalDNS(gslb)
  • controller tests

GslbAssistant

 - implementis IGslbAssistant interface
 - common functionality for NS1 and Route53 exposes with endpoints IP and CRUD for endpoints
 - accepts Logger interface and could log
 - `GetGSLBIngressIPs` -> `GslbIngressExposedIPs`
 - `EnsureDNSEndpoint` -> `SaveDNSEndpoint`
 - add `RemoveEndpoint`

IDNSProvider
 - implementation Route53, NS1
 - having two functions (`HandleZoneDelegation`, `Finalize`)
 - implement `ExternalDNSProvider`

Create private functions in `providers/dns` package
 - nsServerName
 - nsServerNameExt
 - getExternalClusterHeartbeatFQDNs
 - cover all of them by tests

GslbAssistant

 - implementis IGslbAssistant interface
 - common functionality for NS1 and Route53 exposes with endpoints IP and CRUD for endpoints
 - accepts Logger interface and could log
 - `GetGSLBIngressIPs` -> `GslbIngressExposedIPs`
 - `EnsureDNSEndpoint` -> `SaveDNSEndpoint`
 - add `RemoveEndpoint`
 - checkTXTRecord moved to assistant, fixed tests; renamed to InspectTXTThreshold
 - remove gslb parameter from assistant.RemoveEndpoint
 - remove error output argument from GetExternalTargets
 - overrideWithFakeDNS moved to assistant as private func

IDNSProvider
 - implementation Route53, NS1
 - having two functions (`HandleZoneDelegation`, `Finalize`)
 - implement `ExternalDNSProvider`
 - DNSProvider.SaveDNSEndpoint instead of ensureDNSEndpoint

ExternalDNS:
 -  move CreateZoneDelegationForExternalDNS to ExternalDNS
 -  fixed, missing finalizer for NS1 (#262)
 -  fix tests, If config is modified, new provider needs to be re-created. There is no way and reason to change provider
    configuration at another time than startup

Infoblox:
 - split infoblox logic into three package files
 - integrate Infoblox, audit logger fix, no log.Info(fmt.Srintf(...)) needed now, audit.Info(..) takes care..
 - InfobloxDNSProvider contains InfobloxConnection; fakeInfobloxConnection

EmptyDNS
 - `Finalize` runs this: `p.assistant.RemoveEndpoint(gslb.Name)`
 - `CreateZoneDelegationForExternalDNS` returns nil;
 - The rest is implemented the very same way as ExternalDNSPRovider

DNS package
 - TestCanGenerateExternalHeartbeatFQDNs moved to dns package
 - TestCanFilterOutDelegatedZoneEntryAccordingFQDNProvided moved from controller-tests to infoblox
 - TestGeneratesProperExternalNSTargetFQDNsAccordingToTheGeoTags moved to common_test but no completely refactored

Other:
 - deprecated NewGone() to NewResourceExpired
 - implement Stringer interface by providers (now fmt.Sprintf(%s,providerInstance) is writing meaningful message)

Factory implementation
Factory tests
Integrate factory with the rest of the code
 - changes in main.go, instantiate IDNSProvider
 - dnsupdate.go, remove CreateZoneDelegationForExternalDNS
 - controller, using r.DNSProvider.CreateZoneDelegationForExternalDNS(gslb)
 - controller tests
@kuritka kuritka force-pushed the refactor-providers branch from 168a2f3 to bd1a964 Compare January 28, 2021 15:55
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

Refactoring details were presented to the team and the test k8gb build was e2e tested. All good, amazing job @kuritka !

@kuritka kuritka merged commit f1f2ac9 into master Jan 29, 2021
@kuritka kuritka mentioned this pull request Feb 2, 2021
@kuritka kuritka deleted the refactor-providers branch February 2, 2021 07:20
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.

2 participants