Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HCP: error handling & empty source image id #90

Merged
merged 2 commits into from
Dec 16, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions packer/registry/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ func (i *Image) Validate() error {
return errors.New("error registry image does not contain a valid ProviderName")
}

if i.SourceImageID == "" {
return errors.New("error registry image does not contain a valid SourceImageID")
}

azr marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand All @@ -70,7 +66,6 @@ func (i *Image) String() string {
// calling f(k,v). The func f is responsible for type asserting the expected type for the key and value before
// trying to create an Image from it.
func FromMappedData(mappedData interface{}, f func(key, value interface{}) (*Image, error)) ([]*Image, error) {

mapValue := reflect.ValueOf(mappedData)
if mapValue.Kind() != reflect.Map {
return nil, errors.New("error the incoming mappedData does not appear to be a map; found type to be" + mapValue.Kind().String())
Expand Down Expand Up @@ -118,10 +113,6 @@ func FromArtifact(a packer.Artifact, opts ...ArtifactOverrideFunc) (*Image, erro
// used to override the ProviderName for an existing Image.
func WithProvider(name string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.ProviderName = name
return nil
}
Expand All @@ -131,10 +122,6 @@ func WithProvider(name string) func(*Image) error {
// used to override the ImageId for an existing Image.
func WithID(id string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}
Comment on lines -134 to -136
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was added to avoid a panic in case FromMappedData failed and returned a nil image. In this case, I think that this should panic, since the error was not checked. Also, this error is kind of weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ditto throughout)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm also super open to suggestions!)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is weird but panicking too. Maybe improving the error message is better 🤔 what do you think? Making it a friendly panic 😆


img.ImageID = id
return nil
}
Expand All @@ -144,10 +131,6 @@ func WithID(id string) func(*Image) error {
// used to override the SourceImageId for an existing Image.
func WithSourceID(id string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.SourceImageID = id
return nil
}
Expand All @@ -157,10 +140,6 @@ func WithSourceID(id string) func(*Image) error {
// used to override the ProviderRegion for an existing Image.
func WithRegion(region string) func(*Image) error {
return func(img *Image) error {
if img == nil {
return errors.New("no go on empty image")
}

img.ProviderRegion = region
return nil
}
Expand Down