[GPCAPIM-260]-[Steel Thread integration testing]-[RP] #83
Conversation
BJSS-russell-pollock
changed the title
|
✅ Trivy gate: no Critical/High issues. Trivy IaC (Terraform) Summary
Findings (top 50)
|
|
✅ Trivy gate: no Critical/High vulnerabilities. Trivy Image Scan SummaryImage: 900119715266.dkr.ecr.eu-west-2.amazonaws.com/whoami:feature-gpcapim-260
Findings (top 50)
|
BJSS-russell-pollock
force-pushed
the
feature/GPCAPIM-260
branch
4 times, most recently
from
0279c0f to
5fc7fed
Compare
| target_base: ClassVar[str] = "" | ||
| cert: ClassVar[tuple[str, str] | None] = None |
There was a problem hiding this comment.
Should these attributes be class variables here?
There was a problem hiding this comment.
Is it worth naming these attributes with an underscore to highlight that they are private to the class also?
There was a problem hiding this comment.
Changed that
| MtlsProxyHandler.target_base = base_url | ||
| MtlsProxyHandler.cert = get_mtls_cert() |
There was a problem hiding this comment.
Rather than needing to set these class variables on the proxy handler itself, would it be cleaner to instead instantiate an instance of MtlsProxyHandler and provide these values in the constructor? I'm not sure if this created instance can then be provided to HTTPServer below however?
There was a problem hiding this comment.
Changed that
| self, base_url: str, hostname: str | ||
| ) -> None: | ||
| """Verify that the provider satisfies all consumer contracts. | ||
| server.shutdown() |
There was a problem hiding this comment.
Will this shutdown call also handle terminating the created thread?
There was a problem hiding this comment.
Yes. server.serve_forever() is running inside that thread. server.shutdown() signals the server to stop that loop.
| content_length = int(content_length_header) if content_length_header else 0 | ||
| body = self.rfile.read(content_length) if content_length > 0 else None |
There was a problem hiding this comment.
I'm not sure that a content-length header always needs to be included when sending a body via HTTP. So long as the pact library always includes this header though this should be fine here.
There was a problem hiding this comment.
Pact Verifier generates standard JSON payloads for contract tests. It calculates the body size and sends the Content-Length header explicitly. For our current needs, relying on Content-Length in this proxy is safe.
BJSS-russell-pollock
force-pushed
the
feature/GPCAPIM-260
branch
2 times, most recently
from
9e48075 to
1c54c91
Compare
BJSS-russell-pollock
force-pushed
the
feature/GPCAPIM-260
branch
3 times, most recently
from
8d03c82 to
b3f8fb6
Compare
BJSS-russell-pollock
force-pushed
the
feature/GPCAPIM-260
branch
from
b3f8fb6 to
35cbd2f
Compare
|
Deployment Complete
|
williamjameswillis
left a comment
williamjameswillis
left a comment
There was a problem hiding this comment.
approved now comments resolved
3 participants
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.