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

xDS generators should require resource names #2538

Closed
jpeach opened this issue Aug 10, 2021 · 8 comments
Closed

xDS generators should require resource names #2538

jpeach opened this issue Aug 10, 2021 · 8 comments
Labels
good first issue Good for newcomers triage/accepted The issue was reviewed and is complete enough to start working on it

Comments

@jpeach
Copy link
Contributor

jpeach commented Aug 10, 2021

Summary

The xDS generators do not require xDS resources to be named, and there's no checking for that until you get to the consistency snapshot.

Code that looks like it should do nothing will generate anonymous resources, e.g:

b := envoy_routes.NewRouteConfigurationBuilder(envoy.APIV3)
r, err := b.Build()

// r doesn't have a resource name here ...

There are cases where its OK to generate xDS resources that don't have names, but maybe it's more robust to always require them.

Discuss :)

@github-actions
Copy link
Contributor

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Nov 23, 2021
@lahabana lahabana added triage/pending This issue will be looked at on the next triage meeting and removed triage/stale Inactive for some time. It will be triaged again labels Jun 29, 2022
@jakubdyszkiewicz
Copy link
Contributor

Triage
Option 1: b.Build(name)
Option 2: validate that the name is set in .Build(), but this is a runtime error instead of build error

@jakubdyszkiewicz jakubdyszkiewicz added triage/accepted The issue was reviewed and is complete enough to start working on it and removed triage/pending This issue will be looked at on the next triage meeting labels Aug 22, 2022
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Nov 21, 2022
@github-actions
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@jakubdyszkiewicz jakubdyszkiewicz removed the triage/stale Inactive for some time. It will be triaged again label Nov 23, 2022
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Feb 22, 2023
@github-actions
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@michaelbeaumont michaelbeaumont removed the triage/stale Inactive for some time. It will be triaged again label Feb 22, 2023
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label May 24, 2023
@github-actions
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@github-actions
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@lukidzi lukidzi removed the triage/stale Inactive for some time. It will be triaged again label Sep 11, 2023
@github-actions github-actions bot added the triage/stale Inactive for some time. It will be triaged again label Dec 12, 2023
Copy link
Contributor

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed.
If you think this issue is still relevant, please comment on it or attend the next triage meeting.

@jakubdyszkiewicz
Copy link
Contributor

Seems like all builders in pkg/xds/envoy requires names now. Closing

@jakubdyszkiewicz jakubdyszkiewicz removed the triage/stale Inactive for some time. It will be triaged again label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers triage/accepted The issue was reviewed and is complete enough to start working on it
Projects
None yet
Development

No branches or pull requests

5 participants