Ensure ETag is quoted per spec and clean up redundant COA metadata headers - #72
Conversation
…aders Signed-off-by: sanjujunnuthula <sanjuvardhan.junnuthula@capgemini.com>
sanjujunnuthula
changed the title
| deviceVendorLogger.InfofCtx(pCtx, "Set response headers directly - ETag: %s", actualDigest) | ||
| } else { | ||
| deviceVendorLogger.WarnfCtx(pCtx, "Could not access fasthttp context to set headers") | ||
| } |
There was a problem hiding this comment.
seems this block got duplicated, can we have single function which get called for both API routes?
|
|
||
| builder := NewResponseBuilder(span). | ||
| WithContentType(contentType). | ||
| WithMetadata(metadata). |
There was a problem hiding this comment.
The createSuccessResponseWithHeaders function is shared/common in helpers.go, but the PR modifies it to always clear metadata. Is this intentional? Any impact to other endpoints?
There was a problem hiding this comment.
Yes, this is intentional, as createSuccessResponseWithHeaders helper is been only used only bundle and deployment download functions, it doesnt impact other end points
There was a problem hiding this comment.
@sanjujunnuthula If that is the case why can't you send nil instead of headers, highlighted block should be nil, which you are setting explicitly outside function call,
map[string]string{
"Content-Type": "application/yaml",
"Cache-Control": "public, max-age=31536000, immutable",
"ETag": fmt.Sprintf(""%s"", actualDigest), // Quoted ETag
"Vary": "Accept-Encoding",
},
return createSuccessResponseWithHeaders(span,
"application/yaml",
nil,
v1alpha2.OK,
&yamlContent,
)
| @@ -900,6 +913,18 @@ func (self *DeviceAgentVendor) downloadDeployment(request v1alpha2.COARequest) v | |||
| "Serving deployment %s with verified digest %s (%d bytes)", | |||
| deploymentId, actualDigest, len(yamlContent)) | |||
There was a problem hiding this comment.
Don't modify createSuccessResponseWithHeaders at all. Just set headers directly on fasthttp and don't pass metadata:
Signed-off-by: sanjujunnuthula <sanjuvardhan.junnuthula@capgemini.com>
Signed-off-by: sanjujunnuthula <sanjuvardhan.junnuthula@capgemini.com>
spulkit138
left a comment
spulkit138
left a comment
There was a problem hiding this comment.
Looks Good! Can be merged if tested.
3 participants
This PR resolves an issue with the
ETagHTTP response header formatting on the bundle and deployment download endpoints (/clients/{clientId}/bundles/{digest}and/clients/{clientId}/deployments/{deploymentId}/{digest}) to comply with the Margo API specificationIt also cleans up duplicate header serializations in
Coa_meta_header."sha256:<digest>") indevice-agent-vendor.goandhelpers.goto match the API specification ("Quoted digest of the returned bundle archive").Coa_meta_header: Cleaned up the metadata mapping increateSuccessResponseWithHeadersso that standard HTTP headers are not duplicated as a JSON blob insideCoa_meta_header.