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

extract bluecat api code into its own package #2696

Merged
merged 8 commits into from
Apr 12, 2022

Conversation

vinny-sabatini
Copy link
Contributor

@vinny-sabatini vinny-sabatini commented Apr 8, 2022

Description

This was a pretty significant overhaul to the Bluecat provider codebase including:

  • Cleaning up the Bluecat provider code by extracting the code that interacts with the Bluecat Gateway API into its own package
  • Fixing bugs around txt record prefix and suffix that prevented CName records from being created, as well as cname and host records from being deleted if a prefix or suffix was used
  • Do not execute DNS deployments if running in dry-run mode
  • Improves some code quality issues including:
    • Some unit tests for the Gateway API code
    • Consistency when making HTTP request
    • Better error handle HTTP requests (check for errors and verify status codes)

Fixes #2187

Checklist

  • Unit tests updated
  • End user documentation updated

Extract all Bluecat Gateway Client code to separate package
Check status codes when API calls are made for gets and deletes. All of
the creates were done slightly different and will be updated in a
separate commit.
The prefix and suffix for the txt record was not being taken into
account when trying to find an owner for a cname and host record.

In addition, CName records have to be unique values, therefore txt records need to
have either a prefix or suffix included. This fixes the "owner" logic to
ensure when checking a corresponding txt recod against a cname record
that the prefix or suffix is used when comparing records.

Also, correct the response codes for the DELETE calls to Bluecat Gateway
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 8, 2022
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 8, 2022
@vinny-sabatini
Copy link
Contributor Author

/assign @seanmalloy

I would recommend reviewing this PR commit by commit as this has a lot of refactoring

Consolidating the HTTP requests into a function will help ensure
consistency when making HTTP requests to the Bluecat gateway, and in a
future commit will make mocking these requests easier
* Move tests to appropriate package
* Add test for SplitProperties function
* Remove unused helper function
* Use table driven tests
@seanmalloy
Copy link
Member

/kind cleanup
/kind bug

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/bug Categorizes issue or PR as related to a bug. labels Apr 12, 2022
@seanmalloy
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 12, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: seanmalloy, vinny-sabatini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2022
@k8s-ci-robot k8s-ci-robot merged commit 1dcc89e into kubernetes-sigs:master Apr 12, 2022
@vinny-sabatini vinny-sabatini deleted the issue-2187 branch April 12, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bluecat: refactor gateway api REST calls into separate package
3 participants