Set buildpack version as header on dcl upload#93
Conversation
There was a problem hiding this comment.
I don't fully understand the approach here. There is a setter, that is always called und gets the value from the manifest. It does not look clean to me. I see better ways to achieve that. I did not think it trough yet but. I have two ideas:
- Let the uploader read the version from the manifest directly
- Or better: parameterize the User-Agent Header value for the uploader (for example add it to the Uploader struct type)
|
Originally I wanted to use build flags, which however is not possible with the buildpack packager. I thought about adding it to the Uploader struct aswell. I didn't like it either, because you would need to pass it also to all other structs if required somewhere else. Adding the whole manifest to the uploader seems even worse, because the uploader should not even need to know that it is running in the context of a buildpack. Since the project is rather small and we do not have HTTP calls anywhere else outside of the uploader, I'll add it to the Uploader struct for now. |
pkg/supply/supply.go
Outdated
| Root: path.Join(s.Stager.BuildDir(), rootDir), | ||
| Client: client, | ||
| AMSInstanceID: creds.AmsInstanceID, | ||
| UserAgent: fmt.Sprintf("cloud-authorization-buildpack/%s", buildpackVersion), |
There was a problem hiding this comment.
| UserAgent: fmt.Sprintf("cloud-authorization-buildpack/%s", buildpackVersion), | |
| UserAgent: fmt.Sprintf("cloud-authorization-buildpack/%s", s.BuildpackVersion), |
There was a problem hiding this comment.
no need to make to provide an attribute as a parameter to a private funtion.
pkg/supply/supply.go
Outdated
| } | ||
|
|
||
| func (s *Supplier) upload(creds *services.IASCredentials, tlsCfg tlsConfig, rootDir string) error { | ||
| func (s *Supplier) upload(creds *services.IASCredentials, tlsCfg tlsConfig, rootDir, buildpackVersion string) error { |
There was a problem hiding this comment.
| func (s *Supplier) upload(creds *services.IASCredentials, tlsCfg tlsConfig, rootDir, buildpackVersion string) error { | |
| func (s *Supplier) upload(creds *services.IASCredentials, tlsCfg tlsConfig, rootDir) error { |
pkg/supply/supply.go
Outdated
| } | ||
| if cfg.ShouldUpload { | ||
| if err := s.upload(identityCreds, tlsCfg, cfg.Root); err != nil { | ||
| if err := s.upload(identityCreds, tlsCfg, cfg.Root, s.BuildpackVersion); err != nil { |
There was a problem hiding this comment.
| if err := s.upload(identityCreds, tlsCfg, cfg.Root, s.BuildpackVersion); err != nil { | |
| if err := s.upload(identityCreds, tlsCfg, cfg.Root); err != nil { |
2 participants
better traceability which versions are used.