Added GET GatewayService endpoint, tests for it #347

IvanKolchanov · 2024-07-12T09:33:11Z

Had to Add TerminatingGatewayConfigEntry and IngressGatewayConfigEntry classes for testing purposes
Resolves #315

Had to Add TerminatingGatewayConfigEntry and IngressGatewayConfigEntry classes for testing purposes

IvanKolchanov · 2024-07-12T09:39:09Z

@marcin-krystianc should be a working version of #321

marcin-krystianc

Nicely done! Just left a comment that we've already a configuration.cs file with all possible config structures defined.

marcin-krystianc · 2024-07-15T10:13:04Z

Consul/Catalog.cs

+        public bool FromWildcard { get; set; }
+    }
+
+    public class TerminatingGatewayConfigEntry : IConfigurationEntry


We already have a TerminalGatewayEntry in the Configuration.cs file so there is no need to re-define it.
I think the TerminalGatewayEntry and associated LinkedService are incorrectly defined, but feel free to fix them.
It will break backward compatibility, but those structs are likely not used by anyone since they are not correctly defined.

Oh, I am surprised how I missed it. Some of the classes were a bit incorrect. TerminatingGatewayEntry had a wrong name, LinkedService has wrong class structure, TerminatingGatewayEntry had excess fields. IngressGatewayEntry wasn't implementing IConfigEntry. Also I could rename them to TerminatingGatewayConfigEntry and IngressGatewayConfigEntry as in Golang client code, but that isn't critical.

…ed Configuration.cs to have correct classes Changed name TerminalGatewayEntry to TerminatingGatewayEntry

IvanKolchanov · 2024-07-15T13:00:03Z

@marcin-krystianc Tests for backward compatibility fail as expected, but everything else seems to be working.

marcin-krystianc

I think it is almost ready to go, see the comments.

marcin-krystianc · 2024-07-16T06:41:34Z

Consul.Test/CatalogTest.cs

            Assert.DoesNotContain(services.Response.Services, n => n.ID == svcID2);
        }
+
+        [Fact]


That test fails for consul 1.6 and 1.7, you can skip running it for these consul versions: see

consuldotnet/Consul.Test/AgentTest.cs

Lines 542 to 546 in 18a429e

[SkippableFact]

public async Task Agent_MonitorJSON()

{

var cutOffVersion = SemanticVersion.Parse("1.7.0");

Skip.If(AgentVersion < cutOffVersion, $"Current version is {AgentVersion}, but `logjson` is only supported from Consul {cutOffVersion}");

for an example how to do it.

Okey, let me fix that!

marcin-krystianc · 2024-07-16T07:41:36Z

Consul.Test/CatalogTest.cs

+                c.Address = TestHelper.HttpUri;
+            }))
+            {
+                var terminatingGatewayName = "terminating-gateway";


I would use a non-standard name, e.g.:

Suggested change

var terminatingGatewayName = "terminating-gateway";

var terminatingGatewayName = "my-terminating-gateway";

marcin-krystianc · 2024-07-16T07:41:51Z

Consul.Test/CatalogTest.cs

+            }))
+            {
+                var terminatingGatewayName = "terminating-gateway";
+                var ingressGatewayName = "ingress-gateway";


I would use a non-standard name, e.g.:

Suggested change

var ingressGatewayName = "ingress-gateway";

var ingressGatewayName = "my-ingress-gateway";

…4 and earlier Catalog GET GatewayServices is only supported after 1.7.14

marcin-krystianc · 2024-07-16T12:52:03Z

Consul.Test/CatalogTest.cs

+                var gatewayServices = await client.Catalog.GatewayService(terminatingGatewayName);
+
+                Assert.NotEmpty(gatewayServices.Response);
+                Assert.Equal(ServiceKind.TerminatingGateway, gatewayServices.Response[0].GatewayKind);


It would also be good to check if other fields in the response are non-null, hence we know that the struct is correctly defined.

Yep, that's a good idea, let me do that real quick

It would also be good to check if other fields in the response are non-null, hence we know that the struct is correctly defined.

Just added the checks, you can see that we can't check all fields, cause some fields are just not present in the response for both gateways, and some just for ingress one

We can add checks for port and protocol for the ingress one. Apart from that, it looks good.

@marcin-krystianc
Sorry, just saw your message, added the assertions, tests went good

Thanks!

Thanks for help with the PRs!

…nov/consuldotnet into add-gateaway-services

IvanKolchanov and others added 2 commits July 12, 2024 04:29

Added GET GatewayService endpoint, added tests for it

afd02b7

Had to Add TerminatingGatewayConfigEntry and IngressGatewayConfigEntry classes for testing purposes

style: fix whitespaces

8d8b471

marcin-krystianc reviewed Jul 15, 2024

View reviewed changes

marcin-krystianc and others added 2 commits July 15, 2024 12:15

Merge branch 'master' into add-gateaway-services

b50c8b9

Deleted custom Termintaing and Ingress gateway config entries. Modifi…

e70153c

…ed Configuration.cs to have correct classes Changed name TerminalGatewayEntry to TerminatingGatewayEntry

marcin-krystianc reviewed Jul 16, 2024

View reviewed changes

marcin-krystianc marked this pull request as ready for review July 16, 2024 07:50

IvanKolchanov added 2 commits July 16, 2024 03:03

Modified test for List Gateway Services, skippable for versions 1.7.1…

c9305bd

…4 and earlier Catalog GET GatewayServices is only supported after 1.7.14

Using non-standard name for the testing gateway services

954dbf7

marcin-krystianc previously approved these changes Jul 16, 2024

View reviewed changes

marcin-krystianc reviewed Jul 16, 2024

View reviewed changes

marcin-krystianc mentioned this pull request Jul 16, 2024

List services gateway #321

Closed

marcin-krystianc and others added 3 commits July 16, 2024 15:01

Merge branch 'master' into add-gateaway-services

fa04eef

Some field checks for GatewayService results

a80f67f

Merge branch 'add-gateaway-services' of https://github.com/IvanKolcha…

1161a9e

…nov/consuldotnet into add-gateaway-services

IvanKolchanov dismissed marcin-krystianc’s stale review via 1161a9e July 16, 2024 13:31

Minor test change

7705e0a

marcin-krystianc approved these changes Jul 17, 2024

View reviewed changes

marcin-krystianc merged commit b24dd66 into G-Research:master Jul 17, 2024

IvanKolchanov deleted the add-gateaway-services branch July 17, 2024 15:07

dependabot bot mentioned this pull request Nov 8, 2025

Bump Consul from 1.6.10.9 to 1.7.14.9 steingran/MultiLock#13

Closed

	[SkippableFact]
	public async Task Agent_MonitorJSON()
	{
	var cutOffVersion = SemanticVersion.Parse("1.7.0");
	Skip.If(AgentVersion < cutOffVersion, $"Current version is {AgentVersion}, but `logjson` is only supported from Consul {cutOffVersion}");

	var terminatingGatewayName = "terminating-gateway";
	var terminatingGatewayName = "my-terminating-gateway";

	var ingressGatewayName = "ingress-gateway";
	var ingressGatewayName = "my-ingress-gateway";

Added GET GatewayService endpoint, tests for it #347

Added GET GatewayService endpoint, tests for it #347

Uh oh!

Conversation

IvanKolchanov commented Jul 12, 2024

Uh oh!

IvanKolchanov commented Jul 12, 2024

Uh oh!

marcin-krystianc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IvanKolchanov commented Jul 15, 2024

Uh oh!

marcin-krystianc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IvanKolchanov Jul 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IvanKolchanov Jul 17, 2024 •

edited

Loading