Skip to content

matching: disable hcm integration by default#16387

Merged
snowp merged 13 commits intoenvoyproxy:mainfrom
snowp:disable-matching-api
May 12, 2021
Merged

matching: disable hcm integration by default#16387
snowp merged 13 commits intoenvoyproxy:mainfrom
snowp:disable-matching-api

Conversation

@snowp
Copy link
Copy Markdown
Contributor

@snowp snowp commented May 7, 2021

As this is an experimental feature, hide it behind a false by default runtime guard to prevent usage without being
aware of this.

A match tree can still be instantiated via internal APIs, but this should disable the primary way
of enabling this via configuration by default.

Signed-off-by: Snow Pettersen snowp@lyft.com

Risk Level: Low
Testing: UTs
Docs Changes: n/a
Release Notes: Release note added

Snow Pettersen added 6 commits May 7, 2021 22:55
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Looks good, just needs a spelling fix.

}
};

TEST(MatchWrapper, DisbaledByDefault) {
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.

spelling: Disbaled

Snow Pettersen added 2 commits May 8, 2021 00:04
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
antoniovicente
antoniovicente previously approved these changes May 8, 2021
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Thanks.

htuch
htuch previously approved these changes May 9, 2021
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Snow Pettersen added 4 commits May 10, 2021 15:07
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp dismissed stale reviews from htuch and antoniovicente via c3e9472 May 11, 2021 17:14
@snowp snowp requested a review from mattklein123 as a code owner May 11, 2021 17:14
@snowp
Copy link
Copy Markdown
Contributor Author

snowp commented May 11, 2021

@antoniovicente LMK if you're okay with this, I took a look at what it would look like to remove the examples but it requires leaving empty docs pages around to avoid dangling references in protos/release notes so not sure if it's worth it.

Signed-off-by: Snow Pettersen <snowp@lyft.com>
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Looks good.

@snowp snowp merged commit aaebda2 into envoyproxy:main May 12, 2021
@snowp snowp added the backport/approved Approved backports to stable releases label May 12, 2021
@snowp
Copy link
Copy Markdown
Contributor Author

snowp commented May 12, 2021

Marking for backport to 1.8.x (cc @dmitri-d)

htuch pushed a commit that referenced this pull request May 21, 2021
Port of #16387 to 1.18 release branch

Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
Co-authored-by: Snow Pettersen <snowp@lyft.com>
@alyssawilk
Copy link
Copy Markdown
Contributor

checking back in as we cut the release, what's the plan with this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/approved Approved backports to stable releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants