Conversation
de72e32 to
53024e2
Compare
jlocash
left a comment
There was a problem hiding this comment.
Thanks for the PR! I just had one clarifying question
m-horky
left a comment
There was a problem hiding this comment.
I'd like to introduce this new functionality in at least a new file. Long-term, we'll need to properly define a structure somewhere so we can talk to multiple services in a shared manner, but I won't go that far here.
In your commit message, can you include some instructions or even a short script to showcase how this is expected to work? Doesn't have to be pretty, but I'd like a reference to verify against.
More comments inline. I haven't tried the code out yet, these are just a few observations.
53024e2 to
8ed14f9
Compare
|
Steps to verify the functionality:
func main() {
archive := ArchiveDto{
Path: "/var/cache/insights-client/<your-tar-xz-archive>",
ContentType: "application/vnd.redhat.advisor.collection",
}
config := ServiceConfig{
URL: "https://cert.console.redhat.com/api/ingress/v1/upload",
CertPath: "/etc/pki/consumer/cert.pem",
ClientKeyPath: "/etc/pki/consumer/key.pem",
}
err := UploadArchive(archive, config)
if err != nil {
slog.Error(err.Error())
os.Exit(1)
}
}
|
m-horky
left a comment
There was a problem hiding this comment.
It works!
For a tech preview this PR is structured fine, I don't have any major problems with it. I have plans long-term to do dependency injection
type Uploader struct {
httpClient HTTPClient
fs FileSystem
}together with interfaces, which would allow us to test stuff better. The Service would long-term exist as an interface as well, with multiple implementations (Ingress, Inventory, Candlepin) with specific code relevant to the service.
This client isn't communicating what it is, later we'll need to add User-Agent (tracked by CCT-2063).
m-horky
left a comment
There was a problem hiding this comment.
Add upload functionality for collector archive files to the Red Hat Console Ingress service.
It is Red Hat Hybrid Cloud Console, or Red Hat Lightspeed, or Red Hat Insights.
Please include AI sign-off if you have used it.
|
My first question: How do you smoke test this? My expectation would be:
Without some new binary or command you are not able to get feedback from QE soon. I cannot test it too. My feedback could be only from looking at some code that probably work, but it maybe does not work in real work at all and it will be necessary to change in the future. |
That's out of scope. See CCT-1700 (this card) and CCT-1699 (a follow up). Peter provided an example snippet above, which I used for the happy path validation. This is one of several steps we took to make the patch reviewable, and we "paid" with not having product code to verify the new logic. Integration will be handled during CCT-1854 and later through ad-hoc cards addressing the goals of whole feature CCT-1294. |
jirihnidek
left a comment
There was a problem hiding this comment.
I have few comments. I will continue with my review tomorrow.
8ed14f9 to
46f29d8
Compare
jirihnidek
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have several comments and requests.
0feb361 to
1c8b8aa
Compare
Oh, I overlooked this comment. There is lot of comments. I think that this is enough. |
jirihnidek
left a comment
There was a problem hiding this comment.
Thanks for the update. I still have some comments and requests.
1c8b8aa to
78a39d3
Compare
jirihnidek
left a comment
There was a problem hiding this comment.
Thanks for your updates. I have some comments.
78a39d3 to
14b7f20
Compare
m-horky
left a comment
There was a problem hiding this comment.
I like it. My notes are mostly minor, I believe we're getting close to having this PR merged.
* Card ID: CCT-1700 Add upload functionality for collector archive files to the Red Hat Hybrid Cloud Console. The upload flow includes creating multipart form data from archive file, loading client certificates, configuring HTTP client, and sending POST request to ingress endpoint. Co-authored-by: Claude Code
In `internal/http/client.go` there is unused method Get(), which can be safely dropped from the codebase.
14b7f20 to
224cc09
Compare
Add upload functionality for collector archive files to the Red Hat Hybrid Cloud Console. The upload flow includes creating multipart form data from archive file, loading client certificates, configuring HTTP client, and sending POST request to ingress endpoint.
Co-authored-by: Claude Code