Skip to content

SYN-5569: ssl tests client#39

Open
borisvalo wants to merge 5 commits into
splunk:v2from
borisvalo:SYN-5569-ssl-tests-client
Open

SYN-5569: ssl tests client#39
borisvalo wants to merge 5 commits into
splunk:v2from
borisvalo:SYN-5569-ssl-tests-client

Conversation

@borisvalo

Copy link
Copy Markdown
Collaborator

Resolves SYN-5569


Before the change?

  • syntheticsclientv2 did not expose client methods or models for SSL certificate tests.
  • CA certificate /cacerts create/read/update/delete/list support was missing, so downstream consumers could not manage custom CA trust material through the v2 client.

After the change?

  • Added SSL test CRUD methods for /tests/ssl.
  • Added CA certificate create/get/list/update/delete methods for /cacerts.
  • Added request/response models for SSL tests and CA certificates.
  • Added unit test coverage for SSL and CA certificate envelopes, request bodies, blank update responses, and delete status handling.
  • Kept /certificates mTLS/client certificate support out of scope.

Pull request checklist

  • Acceptance Tests have been updated, run (make testacc), and pasted in this PR (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Acceptance Test Output

Not run. This is a syntheticsclient-only change with no live tenant acceptance flow.

Validation run:
GOCACHE=/private/tmp/codex-go-cache GOMODCACHE=/private/tmp/codex-go-mod-cache make test

0 issues.
PASS
coverage: 71.5% of statements
ok github.com/splunk/syntheticsclient/v2/syntheticsclientv2

Does this introduce a breaking change?

  • Yes
  • No

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot CLA Assistant Lite bot All contributors have signed the COC ✍️ ✅

@github-actions

Copy link
Copy Markdown

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

@borisvalo

Copy link
Copy Markdown
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

@borisvalo

Copy link
Copy Markdown
Collaborator Author

I have read the Code of Conduct and I hereby accept the Terms

srv-gh-tools added a commit to splunk/cla-agreement that referenced this pull request Jun 12, 2026
@jnino-splunk

Copy link
Copy Markdown

Review findings:

  • [H1] syntheticsclientv2/common_models.go: CaCertificateV2Response expects a {"cacert": ...} envelope for single CA cert create/get, but the app client models GET /cacerts/{id} as returning the CA certificate object directly. If the API returns the bare object, json.Unmarshal succeeds and leaves resp.CaCert zero-valued, so callers get an apparently successful but empty result. The single-object CA response should unmarshal into CaCertificate directly; keep the list response wrapped as {"cacerts": [...]}.

  • [H2] syntheticsclientv2/common_models.go: SslCheckV2Input.Test.ServerName is a string, so the client cannot send serverName: null. The Synthetics app treats serverName as nullable and explicitly sends null when blank. This client will send "" for the default/no-SNI case, which can be rejected or persisted differently. Use *string for request/response ServerName.

  • [M1] syntheticsclientv2/common_models.go: the SSL response/input models omit mfaCertificateId, even though the app’s SSL model includes it on both read and update config. Even if certificate CRUD is out of scope, updating an SSL test should be able to preserve an existing mTLS/client cert reference; otherwise callers can drop it accidentally.

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