search

GalleyBytes / terraform-operator

A Kubernetes CRD to handle terraform operations
http://tf.galleybytes.com
Apache License 2.0
366 stars 47 forks source link

Mount extra volumes to task pods #158

Closed kepiukik closed 1 year ago

kepiukik commented 1 year ago

Added ability to mount extra k8s volumes (any type) to task pods. Could be useful in pre/post phases.

isaaguilar commented 1 year ago

This is an extension of task options. In reality, a task option can expose most of what a pod exposes, but I'm not sure how opinionated this project should be on that matter. The more that is exposed, the more room for error.

Should volume and volumeMounts be validated that they both have matching names? This would help terraform-operator's controller from producing bad pod configuration.

kepiukik commented 1 year ago

Minimize available task options(pod API's) to prevent extra errors is a good point, but it seems volume mounting is an essential thing.

I can offer 3 options to mitigate risks:

1) Cheap: check if len(volume) == len(volumeMounts) and their names match 2) Medium: all from cheap + fail, if any volume name is equal with any system one (e.g tfo-runner-home) 3) Hard: all from medium + validate mount paths.

In my opinion, 2st option is the best.

isaaguilar commented 1 year ago

I agree that option 2 is the best.

Regards,

Isa Aguilar @.***

From: kepiukik @.> Sent: Thursday, September 21, 2023 6:43:01 AM To: GalleyBytes/terraform-operator @.> Cc: Isa Aguilar @.>; Comment @.> Subject: Re: [GalleyBytes/terraform-operator] Mount extra volumes to task pods (PR #158)

Minimize available task options(pod API's) to prevent extra errors is a good point, but it seems volume mounting is an essential thing.

I can offer 3 options to mitigate risks:

  1. Cheap: check if len(volume) == len(volumeMounts) and their names match
  2. Medium: all from cheap + fail, if any volume name is equal with any system one (e.g tfo-runner-home)
  3. Hard: all from medium + validate mount paths.

In my opinion, 2st option is the best.

— Reply to this email directly, view it on GitHubhttps://github.com/GalleyBytes/terraform-operator/pull/158#issuecomment-1729314856, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD5FPM6KLVQ5IJ7SNPZJNILX3QK3LANCNFSM6AAAAAA46SL6AM. You are receiving this because you commented.Message ID: @.***>

kepiukik commented 1 year ago

Added corresponding checks.