Skip to content

NET-7178: add workload selector to MeshGateway protobuf definition#20159

Merged
NiniOak merged 1 commit intomainfrom
NET-7178_add_workload_selector
Jan 16, 2024
Merged

NET-7178: add workload selector to MeshGateway protobuf definition#20159
NiniOak merged 1 commit intomainfrom
NET-7178_add_workload_selector

Conversation

@NiniOak
Copy link
Copy Markdown
Collaborator

@NiniOak NiniOak commented Jan 10, 2024

Description

This PR updates the MeshGateway protobuf definition to include workload selector. Another PR in Consul-K8s will build upon this to use this field but not add it to the meshgateway CRD

Testing & Reproduction steps

UI test passes
No linting errors

MeshGateway struct updates only. Validate the fields are updated correctly and CI/Lint pass

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@NiniOak NiniOak added theme/mesh-gw Track mesh gateway work pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Jan 10, 2024
@NiniOak NiniOak requested review from a team, andrewstucki and t-eckert and removed request for a team January 10, 2024 23:10
@NiniOak NiniOak requested a review from nathancoleman January 11, 2024 00:15
@NiniOak NiniOak marked this pull request as ready for review January 11, 2024 16:59
@NiniOak NiniOak requested a review from a team as a code owner January 11, 2024 16:59
@NiniOak NiniOak changed the title [WIP] NET-7178: add workload selector to MeshGateway protobuf definition NET-7178: add workload selector to MeshGateway protobuf definition Jan 11, 2024
@NiniOak NiniOak force-pushed the NET-7178_add_workload_selector branch 4 times, most recently from 0fc7c92 to 5279d92 Compare January 12, 2024 16:52
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm based on Nathan's comment here, I think you might need to break this out so it doesn't show up on the CRD. I couldn't find a kubetag that ignores a field completely, but it does look like you can set it to ignore unexported fields

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean we would need to add the ignore option on the controller-gen command itself?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also couldn't find an option to ignore a single field; however, we paired as a group earlier today and worked through omitting the field in the consul-k8s CRD. We'll just use a different parent struct for the CRD in consul-k8s and then map that to a MeshGateway defined here by adding a workload selector before syncing into consul

@NiniOak NiniOak force-pushed the NET-7178_add_workload_selector branch 2 times, most recently from e50a15a to 864cef5 Compare January 12, 2024 18:44
@NiniOak NiniOak requested a review from missylbytes January 12, 2024 18:45
@NiniOak NiniOak force-pushed the NET-7178_add_workload_selector branch 3 times, most recently from 8f2e796 to 2472ac2 Compare January 16, 2024 18:00
Copy link
Copy Markdown
Member

@nathancoleman nathancoleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Some optional commentary below but fine with this merging

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stole the second line from other usages of this type across consul

Suggested change
// Selection of workloads to be exposed through the MeshGateway
// Selection of workloads to be configured as mesh gateways.
// These can be prefixes or specific workload names.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also couldn't find an option to ignore a single field; however, we paired as a group earlier today and worked through omitting the field in the consul-k8s CRD. We'll just use a different parent struct for the CRD in consul-k8s and then map that to a MeshGateway defined here by adding a workload selector before syncing into consul

@NiniOak NiniOak force-pushed the NET-7178_add_workload_selector branch from 8283f73 to 4d3cc05 Compare January 16, 2024 21:28
@NiniOak NiniOak enabled auto-merge (squash) January 16, 2024 21:30
@NiniOak NiniOak merged commit 6a85543 into main Jan 16, 2024
@NiniOak NiniOak deleted the NET-7178_add_workload_selector branch January 16, 2024 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry theme/mesh-gw Track mesh gateway work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants