V3 Work by dcarbone · Pull Request #48 · dcarbone/php-consul-api

dcarbone · 2025-04-11T01:42:01Z

Goals:

Copilot

Copilot reviewed 32 out of 51 changed files in this pull request and generated 2 comments.

Files not reviewed (19)

src/ACL/ACLAuthMethod.php: Language not supported
src/ACL/ACLAuthMethodListEntry.php: Language not supported
src/ACL/ACLAuthMethodListEntryQueryResponse.php: Language not supported
src/ACL/ACLAuthMethodNamespaceRule.php: Language not supported
src/ACL/ACLAuthMethodQueryResponse.php: Language not supported
src/ACL/ACLAuthMethodWriteResponse.php: Language not supported
src/ACL/ACLBindingRule.php: Language not supported
src/ACL/ACLBindingRuleQueryResponse.php: Language not supported
src/ACL/ACLBindingRuleWriteResponse.php: Language not supported
src/ACL/ACLBindingRulesQueryResponse.php: Language not supported
src/ACL/ACLClient.php: Language not supported
src/ACL/ACLEntriesResponse.php: Language not supported
src/ACL/ACLEntry.php: Language not supported
src/ACL/ACLLink.php: Language not supported
src/ACL/ACLLoginParams.php: Language not supported
src/ACL/ACLNodeIdentity.php: Language not supported
src/ACL/ACLOIDCAuthURLParams.php: Language not supported
src/ACL/ACLOIDCCallbackParams.php: Language not supported
src/ACL/ACLPolicy.php: Language not supported

Comments suppressed due to low confidence (1)

README.md:106

There appears to be an extra closing bracket in the client instantiation, which may lead to a syntax error in the example code. Consider removing the extra bracket.

$proxyClient = new \GuzzleHttp\Client(['proxy' => 'whatever proxy you want']]);

README.md

Copilot

Copilot reviewed 41 out of 60 changed files in this pull request and generated 1 comment.

Files not reviewed (19)

src/ACL/ACLAuthMethod.php: Language not supported
src/ACL/ACLAuthMethodListEntry.php: Language not supported
src/ACL/ACLAuthMethodListEntryQueryResponse.php: Language not supported
src/ACL/ACLAuthMethodNamespaceRule.php: Language not supported
src/ACL/ACLAuthMethodQueryResponse.php: Language not supported
src/ACL/ACLAuthMethodWriteResponse.php: Language not supported
src/ACL/ACLBindingRule.php: Language not supported
src/ACL/ACLBindingRuleQueryResponse.php: Language not supported
src/ACL/ACLBindingRuleWriteResponse.php: Language not supported
src/ACL/ACLBindingRulesQueryResponse.php: Language not supported
src/ACL/ACLClient.php: Language not supported
src/ACL/ACLEntriesResponse.php: Language not supported
src/ACL/ACLEntry.php: Language not supported
src/ACL/ACLLink.php: Language not supported
src/ACL/ACLLoginParams.php: Language not supported
src/ACL/ACLNodeIdentity.php: Language not supported
src/ACL/ACLOIDCAuthURLParams.php: Language not supported
src/ACL/ACLOIDCCallbackParams.php: Language not supported
src/ACL/ACLPolicy.php: Language not supported

Comments suppressed due to low confidence (1)

README.md:106

An extra closing bracket appears in the instantiation of \GuzzleHttp\Client, which may result in a syntax error. Please remove the additional bracket.

$proxyClient = new \GuzzleHttp\Client(['proxy' => 'whatever proxy you want']]);

README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

Copilot

Pull request overview

Copilot reviewed 158 out of 555 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/ACL/ACLAuthMethodQueryResponse.php

Copilot · 2026-04-09T23:43:19Z

src/ConfigEntry/MeshGatewayConfig.php

+
+    public function jsonSerialize(): \stdClass
+    {
+        $out = $this->_startJsonSerialize();


jsonSerialize() currently returns an empty object and does not include the Mode field at all, which will silently drop data on encode. Serialize Mode (likely as $this->Mode->value, consistent with other enum-backed fields) and omit it only when it is MeshGatewayMode::Default if that's the desired behavior.

Suggested change

$out = $this->_startJsonSerialize();

$out = $this->_startJsonSerialize();

if (MeshGatewayMode::Default !== $this->Mode) {

$out->Mode = $this->Mode->value;

}

Copilot · 2026-04-09T23:43:20Z

src/ConfigEntry/GatewayTLSConfig.php

+            } elseif ('CipherSuites' === $k || 'cipher_suites' === $k) {
+                $n->setcipherSuites(...$v);
+            } elseif ('SDS' === $k) {


Method call uses the wrong casing (setcipherSuites) but the class defines setCipherSuites. In PHP this is a fatal error at runtime. Rename the call to setCipherSuites(...$v).

src/ConfigEntry/InstanceLevelRateLimits.php

Copilot · 2026-04-09T23:43:20Z

src/ConfigEntry/LoadBalancer.php

+            } elseif ('HashPolicies' === $k || 'hash_policies' === $k) {
+                if (null !== $v) {
+                    foreach ($v as $hp) {
+                        $n->HashPolicies[] = HashPolicy::jsonUnserialize($hp);
+                    }
+                }


$HashPolicies is declared as null|array and defaults to null, but jsonUnserialize() appends to it without initializing it to []. Appending to null will throw. Initialize $n->HashPolicies = []; before the inner loop (or call setHashPolicies(...)) when $v is non-null.

Copilot · 2026-04-09T23:43:21Z

src/ConfigEntry/ServiceResolverRedirect.php

+        if ('' !== $this->ServiceSubset) {
+            $out->service_subset = $this->ServiceSubset;
+        }


JSON output for ServiceSubset and SamenessGroup uses snake_case keys (service_subset, sameness_group) while most other fields in this model are serialized with Consul's usual PascalCase (e.g., Service, Namespace, Datacenter). Earlier versions of this model (per removed FIELDS) also used ServiceSubset. Unless the API explicitly requires snake_case here, this change is likely a breaking change for consumers and/or Consul compatibility. Prefer consistent key casing (and if you need to accept both on input, keep the dual-handling in jsonUnserialize).

Copilot · 2026-04-09T23:43:21Z

src/ConfigEntry/ServiceResolverRedirect.php

+        if ('' !== $this->SamenessGroup) {
+            $out->sameness_group = $this->SamenessGroup;
+        }


JSON output for ServiceSubset and SamenessGroup uses snake_case keys (service_subset, sameness_group) while most other fields in this model are serialized with Consul's usual PascalCase (e.g., Service, Namespace, Datacenter). Earlier versions of this model (per removed FIELDS) also used ServiceSubset. Unless the API explicitly requires snake_case here, this change is likely a breaking change for consumers and/or Consul compatibility. Prefer consistent key casing (and if you need to accept both on input, keep the dual-handling in jsonUnserialize).

Copilot · 2026-04-09T23:43:21Z

src/ConfigEntry/ServiceResolverSubset.php

+        foreach ((array)$decoded as $k => $v) {
+            if ('only_passing' === $k) {
+                $n->OnlyPassing = $v;
+            } else {
+                $n->{$k} = $v;
+            }
+        }


The legacy field name for this property was OnlyPassing (PascalCase). jsonUnserialize() now only maps only_passing, so payloads containing OnlyPassing will no longer populate the property. If Consul returns/accepts OnlyPassing here (consistent with many other endpoints), this will break decoding. Consider accepting both (OnlyPassing and only_passing) similarly to other models in this PR.

Copilot · 2026-04-09T23:43:21Z

src/Agent/AgentHealthServicesResponse.php

+        foreach ($checkInfos as $checkInfo) {
+            $this->AgentServiceChecksInfos[] = AgentServiceChecksInfo::jsonUnserialize($checkInfo);
        }
        $this->Err = $err;


This class no longer uses the ErrorContainer trait (which previously defined $Err), but it still assigns $this->Err. If AbstractResponse does not define $Err, this will create a dynamic property (deprecated in PHP 8.2+ and potentially a runtime error depending on settings). Declare public null|Error $Err = null; here, or ensure the parent class defines it explicitly.

README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

working on v3

cc0831e

dcarbone changed the title ~~working on v3~~ V3 Work Apr 11, 2025

dcarbone added 7 commits April 10, 2025 20:53

bit of readme update

480e352

readme

50e6268

allowing old map-style construction

c1b7373

small readme, may change...

97dfca7

lets be correct.

1a89930

reduce pointless line changes in PR.

9519d43

well that was dumb.

7a4f1c8

dcarbone requested a review from Copilot April 11, 2025 02:30

Copilot AI reviewed Apr 11, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

dcarbone added 4 commits April 10, 2025 22:04

removing trash, updating some base classes

f847aee

it can't be null

838eda5

Fine.

65e8e85

grammar

0301bec

dcarbone requested a review from Copilot April 11, 2025 03:12

Copilot AI reviewed Apr 11, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

dcarbone and others added 10 commits April 10, 2025 22:13

dangit, the boy is right.

99f810d

thanx fren.

7b5bb04

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

allow default in $data, starting work on Agent

279b0fb

oops

13784d3

lots of little work, long ways to go.

f53867c

lots more work, need to go back and fix some things.

6e11429

missed one

cbec949

maybe agent done?

af356ca

i dunno, sure.

f047f4c

ugh.

2554c66

dcarbone self-assigned this Apr 14, 2025

dcarbone added 2 commits April 14, 2025 19:56

ACL work.

4f1a05c

some cleanup

ec161b4

dcarbone added 24 commits April 3, 2026 15:11

more cleanup

3ed52eb

lots of little fixes

55725ed

buncha fixes

4d9c5ec

buncha tests

5898715

unit tests, but will reqiure several rounds of fixes.

f5c842f

long way to go yet.

23a684c

some kv fixes

064a7e5

acl stuff

2a94e5f

agent stuff

0c602dc

catalog stuff

4101bd5

config entry stuff

f7f314e

unset is a bad time.

48709f1

coordinate tests

899fd22

coordinate tests

dd9ee20

event

62e8a78

health stuff

f3d5269

kv stuff

593c913

more unit tests

8917f86

more unit tests & stuff

b81c2e1

more tests!

ff9746e

maybe fixing stuff

0497408

sure.

d8d617d

mebe fixes

d849c79

display deprecations

b35add1

dcarbone requested a review from Copilot April 9, 2026 23:40

Copilot AI reviewed Apr 9, 2026

View reviewed changes

dcarbone and others added 4 commits April 9, 2026 18:46

stuff

b3a37a6

Update src/ACL/ACLAuthMethodQueryResponse.php

54d31b7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

Update src/ConfigEntry/InstanceLevelRateLimits.php

e150575

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

Update README.md

70c0c79

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Daniel Carbone <daniel.p.carbone@gmail.com>

Uh oh!

Conversation

dcarbone commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goals:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dcarbone commented Apr 11, 2025 •

edited

Loading