-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: Add Plural DNS provider #2923
feat: Add Plural DNS provider #2923
Conversation
Welcome @davidspek! |
a187a4d
to
39e7a68
Compare
/assign @njuettner @seanmalloy |
/assign @Raffo |
/assign @szuecs |
go.mod
Outdated
) | ||
|
||
replace k8s.io/klog/v2 => github.com/Raffo/knolog v0.0.0-20211016155154-e4d5e0cc970a | ||
|
||
replace github.com/Yamashou/gqlgenc => github.com/pluralsh/gqlgenc v0.0.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you clarify this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we had to fork this library because it uses some custom JSON parser that throws nil pointer exception when trying to unmarshal our GraphQL schema:
Yamashou/gqlgenc#141
We can try to contribute to this library and fix the issue but it's not simple and can take some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szuecs right now the problem is fixed in the github.com/Yamashou/gqlgenc
library and this line is not needed anymore
provider/plural/client.go
Outdated
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be dropped as there is a CLA you sign.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
func (c *Config) BaseUrl() string { | ||
host := "https://app.plural.sh" | ||
if c.Endpoint != "" { | ||
host = fmt.Sprintf("https://%s", c.Endpoint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better make sure that it's actually an URL, so url.Parse and handle error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
looks fine to me in general some small things (besides the replace directive which should be clear why and documented). |
39e7a68
to
36c9c69
Compare
@szuecs PTAL |
add plural to provider enum go mod tidy remove gitlab-ci Signed-off-by: DavidSpek <vanderspek.david@gmail.com> cleanup Signed-off-by: DavidSpek <vanderspek.david@gmail.com>
Signed-off-by: DavidSpek <vanderspek.david@gmail.com>
043be4a
to
3a5fece
Compare
3a5fece
to
73d7ecd
Compare
73d7ecd
to
0b2d3ed
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DavidSpek, szuecs 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 |
/lgtm |
Description
This PR adds the Plural DNS provider to external DNS.
Background
Plural is a platform that deploys open source applications on Kubernetes in your cloud using common standards like Helm and Terraform.
The Plural platform provides the following:
In addition, Plural also handles:
Checklist