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
884 stars 216 forks source link

Subscription DLQ topic expect fully qualified name #800

Open naresh-gn opened 1 year ago

naresh-gn commented 1 year ago

Checklist

Describe the feature or resource

In PubsubSubcription deadLetterPolicy.deadLetterTopicRef.external expects fully qualified name for the topic projects/project-id/topics/topic-name whereas in other places such as topicRef.external accepts just the resource name (need not be fully qualified) deadLetterPolicy.deadLetterTopicRef.external should allow not fully qualified resource name

Additional information

No response

Importance

need to provide project id while deploying to different projects

diviner524 commented 1 year ago

@naresh-gn Could you please provide a sample YAML that can be used to reproduce this?

i.e. It shows topicRef.external accepts just the resource name.

Our current doc for topicRef.external is: "string of the format projects/{{project}}/topics/{{value}} "

https://cloud.google.com/config-connector/docs/reference/resource-docs/pubsub/pubsubsubscription#spec:~:text=to%20a%20PubSubTopic.-,topicRef.external,-Optional

naresh-gn commented 1 year ago

@diviner524 as you can see in below example topicRef.external accepts just the name even though the doc says the format should be projects/<project id>/topics/<topic name> but where as deadLetterPolicy.deadLetterTopicRef.external only accepts fully qualified name, if you use just topic name it error out with

Error creating Subscription: googleapi: Error 400: Invalid resource name given (dead_letter_topic=my-error-topic)

Having full name makes it difficult promoting code different projects. I understand if you try to use different project topic you need fully qualified name but if you are using topic that belongs in the same project then fully qualified name is not required.

apiVersion: pubsub.cnrm.cloud.google.com/v1beta1
kind: PubSubSubscription
metadata:
  annotations:
    dummy: dummy-value
  name: browser-test-1
  namespace: test
spec:
  ackDeadlineSeconds: 300
  deadLetterPolicy:
    deadLetterTopicRef:
      external: projects/<project Id>/topics/my-error-topic
  messageRetentionDuration: 604800s
  resourceID: my-subscription
  retainAckedMessages: false
  topicRef:
    external: my-topic
diviner524 commented 1 year ago

I see, thanks for sharing the example @naresh-gn.

I will try to explain how the external field works.

The supported format of external is determined by the underlying GCP API, not Config Connector. When a string value is provided in external, it is kept as it is and sent to to corresponding GCP API, in this case the pubsub subscriptions API [1].

As you can see from the API reference page, the description for topic field is:

Required. The name of the topic from which this subscription is receiving messages. Format is projects/{project}/topics/{topic}. 

While the description for deadLetterTopic field is:

The name of the topic to which dead letter messages should be published. Format is projects/{project}/topics/{topic}.

I am not sure why the topic field also supports a different format {topic}, it sounds like an undocumented feature which we should not rely on.

On the other hand, if you don't want to specify the format projects/{project}/topics/{topic}, have you considered managing the deadletter topic as just another KCC resource, and use deadLetterPolicy.deadLetterTopicRef.name to refer to the topic resource?

[1] https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#resource:-subscription

naresh-gn commented 1 year ago

thanks @diviner524 the inconsistence is from API for sure. Is it possible to connect with API team to update the doc and also support {topic}? We can't go by the k8s deployment name because our deployment names changes based on the github branch. We do feature branch development, means each branch gets it own dedicated GCP resources.

diviner524 commented 1 year ago

@naresh-gn Sure we can provide the feedback to pubsub team. You can also open a GCP support case to request this as a new feature of PubSub. Although I doubt if the API will officially document and support two different formats for the same field.

Regarding the dedicated GCP resource problem you mentioned, could you elaborate more on that? This might be possible with the combination of the resourceID feature [1] in Config Connector, but we will need more details on your current solution to make sure.

[1] https://cloud.google.com/config-connector/docs/how-to/managing-resources-with-resource-ids