Skip to content

ESD-1311: Add GetMCRIPsec method - #162

Open
Phil-Browne wants to merge 11 commits into
mainfrom
feature/ESD-1311-mcr-ipsec-get
Open

ESD-1311: Add GetMCRIPsec method#162
Phil-Browne wants to merge 11 commits into
mainfrom
feature/ESD-1311-mcr-ipsec-get

Conversation

@Phil-Browne

@Phil-Browne Phil-Browne commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Adds MCRService.GetMCRIPsec wrapping GET /v3/products/mcrs/{uid}/ipsec — the only way to read IPsec tunnel state back after ordering, which the Terraform provider needs for Read/import/drift detection.

  • New types MCRIPsecConfiguration, IPsecConfiguredVXC, IPsecTunnel per the spec
  • Doc comments note the PSK and phase lifetimes are never returned
  • Unit tests with mocked responses; TestMCRWithIPsecAddOn integration test now reads back IPsec state

Jira: ESD-1311

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread mcr.go
Comment thread mcr_test.go
Comment thread mcr_integration_test.go Outdated
Comment thread mcr.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@Phil-Browne
Phil-Browne requested review from a team and mola-dev and removed request for a team June 3, 2026 13:24
Return ErrMCRIPsecNoData (with trace id) instead of (nil, nil) when a
successful response has no data payload, so Terraform Read/import does
not panic dereferencing a nil configuration.
When LogResponseBody is set, Do drained the network body and swapped in
an in-memory NopCloser, so the caller's deferred Close hit the no-op
copy and the original body was never closed. Close it once drained.
- Assert the 404 test returns the API not-found error, not just any error
- Drive the LogResponseBody test with the populated fixture and assert the
  full decoded struct, so a truncated body would fail it
- Share the populated fixture via a helper to remove duplication
- Drop em dashes and a rot-prone hardcoded limit from comments
@Phil-Browne
Phil-Browne requested a review from a team June 24, 2026 14:48
@Phil-Browne
Phil-Browne removed the request for review from mola-dev June 26, 2026 01:37
The "never includes the pre-shared key or phase lifetimes" note was
repeated across the interface, method, and two type docs. Keep it on
IPsecTunnel, the type that actually lacks the fields.
The client.go hunk was a variable rename and comment rewording with no
behavior change, unrelated to the IPsec read path. Restores the file to
main to keep the diff scoped to GetMCRIPsec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants