GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources
https://cloud.google.com/config-connector/docs/overview
Apache License 2.0
894 stars 228 forks source link

Support referencing IP from to-be-created ComputeAddress #101

Open shogomuranushi opened 4 years ago

shogomuranushi commented 4 years ago

I want to put the value of ComputeAddress in rrdatas of DNSRecordSet.

When I look at the references, it looks like I can't do external references. Is there any good way?

---
apiVersion: compute.cnrm.cloud.google.com/v1beta1
kind: ComputeAddress
metadata:
  name: staticip
spec:
  location: global
---
apiVersion: dns.cnrm.cloud.google.com/v1beta1
kind: DNSRecordSet
metadata:
  name: xxx-recorad
spec:
  name: "xxx.example.com."
  type: "A"
  ttl: 300
  managedZoneRef:
    external: managezone
  rrdatas:
  - "8.8.8.8" # <-- I want to put the IP address created with ComputeAddress here.
AlexBulankou commented 4 years ago

@shogomuranushi , thanks for reporting the issue. At this time it is not possible with Config Connector to reference a value of ComputeAddress's IP address dynamically, so you would need to wait until ComputeAddress is created and update rrdatas then. As part of our larger effort to improve end-to-end provisioning scenarios for IP address we are considering improvements in this area later in 2020; we'll update this thread with more information then.

snuggie12 commented 4 years ago

Will this allow for any kind of reference? We want to be able to reference the IP address of a load balancer created by a k8s service. The load balancer's name is randomized so referencing the k8s service's status.loadBalancer.ingress.ip.0 would have the IP I'm looking for.

thecloudgeek commented 4 years ago

bump on this one...this is similar to this one too...https://github.com/GoogleCloudPlatform/k8s-config-connector/issues/166

kibbles-n-bytes commented 4 years ago

Hey @thecloudgeek ; unfortunately we don't yet have any updates on this front, but we'll keep you posted.

AlexBulankou commented 4 years ago

FYI @mountainsaxena

skeenan947 commented 3 years ago

Is anything going to be done about this? It's a pretty big bit to be missing.

toumorokoshi commented 3 years ago

Hi! It's been a while on this issue, thanks for pinging to show you need it.

I'll talk to the team a bit and come back. Agreed this is a use case that should be supported.

toumorokoshi commented 3 years ago

Hello,

Just a quick update: we've taken a look at right approach, and we believe that it's appropriate to change the schema of the rrdatas list to support an object. Since that would be a backwards-incompatible change, we'd like to pair that with some work to support multiple versions for the same CRD.

We've tentatively set the timeline for sometime near the end of Q1. I'll update this ticket as the timeline becomes more clear.

skeenan947 commented 3 years ago

Thank you! Great to hear.

skeenan947 commented 3 years ago

Any update on the timeline for this? We've got a hacky workaround to this but really need this across a ton of our applications. Thanks!

jcanseco commented 3 years ago

Hi @skeenan947, I apologize that we missed your question. Unfortunately, we don't have a real ETA on a solution yet. We have been keeping the issue in mind, and we have been discussing possible solutions. Our capacity's just been quite tight recently, so we haven't been able to invest the time needed to develop the infrastructure needed to resolve issues like this. Once we make more headway, we'll make sure to let you know. Unfortunately, you'll have to rely on the hacky workaround for the moment.

fujin commented 2 years ago

Any word on this? The ability to set fields referencing the output values of other resources would be excellent.

mbzomowski commented 2 years ago

Hi @fujin, sorry, but still no ETA as of yet, although we are actively working on a way to introduce such changes in a backwards compatible manner.

npitts0811 commented 2 years ago

Any update on this? This would greatly simplify some work we're currently doing.

Edit: As part of this update, could another DNSRecordSet be referenced in rrdatas as well as a ComputeAddress? In other words, where an A record DNSRecordSet would reference a ComputeAddress resource, could a CNAME DNSRecordSet reference an A record DNSRecordSet?

mbzomowski commented 2 years ago

Hi @nathanielpitts-bestbuy this is currently being worked on - we're targeting the end of April for this feature, but it might be out sooner.

As for adding in DNSRecordSet as a reference for the rrdatas field, which field in the referenced DNSRecordSet would this reference map to?

npitts0811 commented 2 years ago

Hi @nathanielpitts-bestbuy this is currently being worked on - we're targeting the end of April for this feature, but it might be out sooner.

awesome! Thanks for the update.

As for adding in DNSRecordSet as a reference for the rrdatas field, which field in the referenced DNSRecordSet would this reference map to?

At least in the case of a CNAME type DNSRecordSet, it should reference the spec name field of the A record DNSRecordSet (which itself would reference the spec address field of a ComputeAddress resource).

Something like this: DNSRecordSet/CNAME : rrdatas -> DNSRecordSet/A record : name DNSRecordSet/A record : rrdatas -> ComputeAddress: address

mbzomowski commented 2 years ago

So rrdatas is a slice/array field. I'm not sure how that would work if you wanted to reference the DNSRecordSet/A record rrdatas field from an element of the rrdatas field in the DNSRecordSet/CNAME resource.

Can you clarify?

npitts0811 commented 2 years ago

Sorry, it doesn't need to come from the rrdatas field. I was simply making an (incorrect) assumption how it would work based on the current CRD. Regardless of how referencing a DNSRecordSet/A resource from a DNSRecordSet/CNAME resource would work, we would definitely find value having that capability. Fwiw, I do think this is lower priority for us than being able to reference a ComputeAddress resource from a DNSRecordSet/A resource.

Speaking of which, we just came across a situation this week that a DNSRecordSet/A resource needs to point to two IP addresses. Today we're handling this with lookups of the ComputeAddress resources, but this does have me wondering if the solution being worked on would be able to handle referencing multiple ComputeAddress resources or just a single one?

mbzomowski commented 2 years ago

@nathanielpitts-bestbuy Yes, since the rrdatasRefs field we're adding is an array field, a DNSRecordSet will be able to reference multiple ComputeAddress resources.

Regardless of how referencing a DNSRecordSet/A resource from a DNSRecordSet/CNAME resource would work, we would definitely find value having that capability.

We'd need clarification first on which field in the referenced DNSRecordSet would be referenced by which field in the parent DNSRecordSet.

npitts0811 commented 2 years ago

@mbzomowski Awesome to hear for referencing multiple ComputeAddress resources.

When referencing another DNSRecordSet resource, the relevant field in the referenced DNSRecordSet would be name. Assuming that rradatasRef would still be used for referencing other DNSRecordSets, it would be similar to what I provided earlier (new version below). Apologies if this assumption is incorrect. I can provide a more detailed example if it would be helpful.

The intent below is for a CNAME record to point to an A record in DNS: DNSRecordSet/CNAME : spec: rrdatasRef -> DNSRecordSet/A record (spec: name)

mbzomowski commented 2 years ago

@nathanielpitts-bestbuy yeah, I do think a more detailed example would be helpful - can you provide example yamls for the two DNSRecordSets?

Also, in this scenario, would the referenced DNSRecordSet's rrdatas field be appended to the parent's rrdatas field?

npitts0811 commented 2 years ago

@mbzomowski I don't think the referenced DNSRecordSet's rradatas field needs to be appended to the parent's rrdatas field. The CNAME and A records are distinct resources within DNS (though related). If that answer doesn't make sense let me know, definitely possibly I misunderstood your question :)

Here's a sample based on the rrdatasRef example on the updated Config Connector documentation for DNSRecordSet for v1.84.0. The first yaml document below is simply the example on that page for an A record referencing a ComputeAddress resource, the second document below is a hypothetical CNAME record using rrdatasRef to reference a DNSRecordSet (A record) resource. Obviously I'm just making a suggestion / taking a guess on how this would actually look once its implemented.

---
apiVersion: dns.cnrm.cloud.google.com/v1beta1
kind: DNSRecordSet
metadata:
  name: dnsrecordset-sample-computeaddressreference
spec:
  name: "www.compute-address-reference-example.com."
  type: A
  ttl: 300
  managedZoneRef:
    name: dnsrecordset-dep-computeaddressreference
  rrdatasRefs:
    - name: dnsrecordset-dep-computeaddressreference
      kind: ComputeAddress
---
apiVersion: dns.cnrm.cloud.google.com/v1beta1
kind: DNSRecordSet
metadata:
  name: dnsrecordset-sample-dnsrecordsetreference
spec:
  name: "my-cname-record.compute-address-reference-example.com."
  type: CNAME
  ttl: 300
  managedZoneRef:
    name: dnsrecordset-dep-computeaddressreference
  rrdatasRefs:
    - name: dnsrecordset-sample-computeaddressreference
      kind: DNSRecordSet

It is probably worth mentioning, as far as DNS is concerned: a single CNAME can only point to a single source record (i.e., A record), however multiple CNAMEs can point to the same source record. I want to point this out because in the case of a CNAME resource definition like above, there should only be a single reference to a DNSRecordSet A record in rrdatasRefs.

diviner524 commented 2 years ago

@nathanielpitts-bestbuy Thank you for the detailed description and the hypothetical CNAME example!

I have a follow up question, what's the user scenario when you need to get a name field from an A type DNSRecordSet and then use it to fill in another CNAME type DNSRecordSet?

One major reason we added support for ComputeAddress in rrdatasRefs is because the IP address of a to-be-created ComputeAddress cannot be determined in advance and thus making it impossible to create an A type DNSRecordSet which points to that unknown IP address. In the CNAME scenario, I assume (please correct me if I am wrong) we will already know the target domain name so we can use that name to fill a CNAME type DNSRecordSet directly? Is that not the case in your scenario?

npitts0811 commented 2 years ago

@diviner524 No problem! Hopefully it was helpful :)

Your assumption is correct, at least in our current case. We do know the target domain name value, so we can use that when creating the CNAME resource. What I didn't know when I asked the question originally (or until just earlier today) is that Cloud DNS allows CNAME records to be created without a valid A record for the target domain name. This means we can continue using rrdatas for creating CNAME resources and once the A record is created in DNS (by referencing the ComputeAddress resource via rrdatasRefs), that CNAME becomes useful for DNS lookups.

That said, I do still think there's value for using rrdatasRefs to reference a DNSRecordSet A record resource when defining a DNSRecordSet CNAME resource. This would ensure we don't have CNAME records published in DNS until the referenced A record is valid. Normally, this is just a matter of seconds, so if the CNAME exists temporarily without the A record, no big deal...but I do worry for cases that the ComputeAddress does not get created for whatever reason, which prevents the A record from getting created, so we end up with an "invalid" CNAME published in DNS for a much longer period of time than intended. Obviously, we would catch this and would fix it, but the big benefit to me is the DNSRecordSet CNAME resource having a reliable dependency on the DNSRecordSet A record resource.

Side question: I did notice on the Config Connector documentation for DNSRecordSet that rrdatas is now marked as deprecated, which surprised me. Is the intent to move everything to rrdatasRefs eventually?

diviner524 commented 2 years ago

@nathanielpitts-bestbuy Thanks for sharing these details in your usage scenario! This is very helpful.

Regarding your side question, sorry for the confusion! We don't have plan to remove rrdatas field in current major version, as that will be a breaking change to customers. The main motivation of marking it as deprecated is to encourage people use rrdatasRefs more. We are making some updates to the documentation/comments on this field, which hopefully will reduce the confusion in future.

whyvez commented 2 years ago

Looks like this has been released but I still cannot figure out how to use it. I keep getting a validation error saying that rrdatasRefs isn't part of the schema. Looks like this is part of the v1beta2 api but not sure how to install.

https://github.com/GoogleCloudPlatform/k8s-config-connector/issues/680

The docs show an example using v1beta1.

apiVersion: dns.cnrm.cloud.google.com/v1beta1
kind: DNSRecordSet
metadata:
  name: dnsrecordset-sample-computeaddressreference
spec:
  name: "www.compute-address-reference-example.com."
  type: A
  ttl: 300
  managedZoneRef:
    name: dnsrecordset-dep-computeaddressreference
  rrdatasRefs:
    - name: dnsrecordset-dep-computeaddressreference
      kind: ComputeAddress
maqiuyujoyce commented 2 years ago

@whyvez thank you for reporting your issue. As you're having a slightly different problem from this one, and have filed a different GH issue (#680), I've followed up there instead.

max-frank commented 2 years ago

Now that ComputeAddress is supported is there a roadmap for adding support for k8s load balancer external IPs as references (i.e., kind: LoadBalancer)?

maqiuyujoyce commented 2 years ago

Hi @max-frank , thanks for your question. As this seems to be slightly different from #101, could you file a new issue with your detailed use case so we can follow up there?

moshikbaruch commented 10 months ago

Hey, is there any update about adding a ref to cloud-sql privateIP here?

snuggie12 commented 5 days ago

Any update on this? Our most pressing resource currently are redis memorystore IPs