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

Support reading environment from secret/configmap #2295

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
117 changes: 116 additions & 1 deletion docs/tenant_crd.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,35 @@ CertificateStatus keeps track of all the certificates managed by the operator
|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-configmapkeyselector"]
==== ConfigMapKeySelector

Selects a key from a ConfigMap.

.Appears In:
****
- xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvarsource[$$EnvVarSource$$]
****

[cols="25a,75a", options="header"]
|===
| Field | Description

|*`name`* __string__
|Name of the referent.
This field is effectively required, but due to backwards compatibility is
allowed to be empty. Instances of this type with an empty value here are
almost certainly wrong.
TODO: Add other useful fields. apiVersion, kind, uid?
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Drop `kubebuilder:default` when controller-gen doesn't need it https://github.com/kubernetes-sigs/kubebuilder/issues/3896.

|*`key`* __string__
|The key to select.

|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-customcertificateconfig"]
==== CustomCertificateConfig

Expand Down Expand Up @@ -190,6 +219,63 @@ Certificate Authorities
|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvar"]
==== EnvVar

EnvVar represents an environment variable present in a Container.

.Appears In:
****
- xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-tenantspec[$$TenantSpec$$]
****

[cols="25a,75a", options="header"]
|===
| Field | Description

|*`name`* __string__
|Name of the environment variable. Must be a C_IDENTIFIER.

|*`value`* __string__
|Variable references $(VAR_NAME) are expanded
using the previously defined environment variables in the container and
any service environment variables. If a variable cannot be resolved,
the reference in the input string will be unchanged. Double $$ are reduced
to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e.
"$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)".
Escaped references will never be expanded, regardless of whether the variable
exists or not.
Defaults to "".

|*`valueFrom`* __xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvarsource[$$EnvVarSource$$]__
|Source for the environment variable's value. Cannot be used if value is not empty.

|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvarsource"]
==== EnvVarSource

EnvVarSource represents a source for the value of an EnvVar.

.Appears In:
****
- xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvar[$$EnvVar$$]
****

[cols="25a,75a", options="header"]
|===
| Field | Description

|*`configMapKeyRef`* __xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-configmapkeyselector[$$ConfigMapKeySelector$$]__
|Selects a key of a ConfigMap.

|*`secretKeyRef`* __xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-secretkeyselector[$$SecretKeySelector$$]__
|Selects a key of a secret in the pod's namespace

|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-exposeservices"]
==== ExposeServices

Expand Down Expand Up @@ -689,6 +775,35 @@ Security Context
|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-secretkeyselector"]
==== SecretKeySelector

SecretKeySelector selects a key of a Secret.

.Appears In:
****
- xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvarsource[$$EnvVarSource$$]
****

[cols="25a,75a", options="header"]
|===
| Field | Description

|*`name`* __string__
|Name of the referent.
This field is effectively required, but due to backwards compatibility is
allowed to be empty. Instances of this type with an empty value here are
almost certainly wrong.
TODO: Add other useful fields. apiVersion, kind, uid?
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Drop `kubebuilder:default` when controller-gen doesn't need it https://github.com/kubernetes-sigs/kubebuilder/issues/3896.

|*`key`* __string__
|The key of the secret to select from. Must be a valid secret key.

|===


[id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-servicemetadata"]
==== ServiceMetadata

Expand Down Expand Up @@ -915,7 +1030,7 @@ Specify the secret key to use for pulling images from a private Docker repositor

Pod Management Policy for pod created by StatefulSet

|*`env`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#envvar-v1-core[$$EnvVar$$] array__
|*`env`* __xref:{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-envvar[$$EnvVar$$] array__
|*Optional* +


Expand Down
30 changes: 0 additions & 30 deletions helm/operator/templates/minio.min.io_tenants.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -901,47 +901,17 @@ spec:
name:
default: ""
type: string
optional:
type: boolean
required:
- key
type: object
x-kubernetes-map-type: atomic
fieldRef:
properties:
apiVersion:
type: string
fieldPath:
type: string
required:
- fieldPath
type: object
x-kubernetes-map-type: atomic
resourceFieldRef:
properties:
containerName:
type: string
divisor:
anyOf:
- type: integer
- type: string
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
resource:
type: string
required:
- resource
type: object
x-kubernetes-map-type: atomic
secretKeyRef:
properties:
key:
type: string
name:
default: ""
type: string
optional:
type: boolean
required:
- key
type: object
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/minio.min.io/v2/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func GetPrivateKeyFilePath(serviceName string) string {
func GetNSFromFile() string {
namespace, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace")
if err != nil {
return "minio-operator"
return os.Getenv("DEV_NAMESPACE")
}
return string(namespace)
}
Expand Down Expand Up @@ -523,7 +523,7 @@ func (t *Tenant) HasPrometheusOperatorEnabled() bool {
}

// GetEnvVars returns the environment variables for tenant deployment.
func (t *Tenant) GetEnvVars() (env []corev1.EnvVar) {
func (t *Tenant) GetEnvVars() (env []EnvVar) {
return t.Spec.Env
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/minio.min.io/v2/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ func TestTenant_HasEnv(t1 *testing.T) {
name: "Contains env",
fields: fields{
Spec: TenantSpec{
Env: []corev1.EnvVar{
Env: []EnvVar{
{
Name: "ENV1",
Value: "whatever",
Expand All @@ -412,7 +412,7 @@ func TestTenant_HasEnv(t1 *testing.T) {
name: "Does not Contains env",
fields: fields{
Spec: TenantSpec{
Env: []corev1.EnvVar{
Env: []EnvVar{
{
Name: "ENV1",
Value: "whatever",
Expand Down
53 changes: 52 additions & 1 deletion pkg/apis/minio.min.io/v2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ type TenantSpec struct {
//
// If provided, the MinIO Operator adds the specified environment variables when deploying the Tenant resource.
// +optional
Env []corev1.EnvVar `json:"env,omitempty"`
Env []EnvVar `json:"env,omitempty"`
pjuarezd marked this conversation as resolved.
Show resolved Hide resolved

// *Optional* +
//
Expand Down Expand Up @@ -910,3 +910,54 @@ type SideCars struct {
// +optional
Resources *corev1.ResourceRequirements `json:"resources,omitempty"`
}

// EnvVar represents an environment variable present in a Container.
type EnvVar struct {
// Name of the environment variable. Must be a C_IDENTIFIER.
Name string `json:"name" protobuf:"bytes,1,opt,name=name"`

// Optional: no more than one of the following may be specified.

// Variable references $(VAR_NAME) are expanded
// using the previously defined environment variables in the container and
// any service environment variables. If a variable cannot be resolved,
// the reference in the input string will be unchanged. Double $$ are reduced
// to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e.
// "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)".
// Escaped references will never be expanded, regardless of whether the variable
// exists or not.
// Defaults to "".
// +optional
Value string `json:"value,omitempty" protobuf:"bytes,2,opt,name=value"`
// Source for the environment variable's value. Cannot be used if value is not empty.
// +optional
ValueFrom *EnvVarSource `json:"valueFrom,omitempty" protobuf:"bytes,3,opt,name=valueFrom"`
ramondeklein marked this conversation as resolved.
Show resolved Hide resolved
}

// EnvVarSource represents a source for the value of an EnvVar.
type EnvVarSource struct {
// Selects a key of a ConfigMap.
// +optional
ConfigMapKeyRef *ConfigMapKeySelector `json:"configMapKeyRef,omitempty" protobuf:"bytes,3,opt,name=configMapKeyRef"`
Copy link
Member

Choose a reason for hiding this comment

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

This could be corev1.ConfigMapKeySelector, I don't see why we would not honor the Optional flag that is being removed

Suggested change
ConfigMapKeyRef *ConfigMapKeySelector `json:"configMapKeyRef,omitempty" protobuf:"bytes,3,opt,name=configMapKeyRef"`
ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty" protobuf:"bytes,3,opt,name=configMapKeyRef"`

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 did that at first, but code is more complicated, because when the value is not optional we need to generate an error and abort the creation. It adds a lot of code. The current implementation just skips the environment variable (so optional is always enabled) if it cannot find the source.

There is no valid use-case to allow optional/non-optional support (AFAIK), so I prefered to use the simple code.

Copy link
Member

Choose a reason for hiding this comment

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

How do we notify that a env variable could not be mounted to the user?, we just silently remove it?

optional could solve this problem, even if means more code, if optional: false then we should error out somewhere, if optional: true we can silently drop the env variable, as the user stated that it is fine to run MinIO without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I just skipped it. I'll rework to support Optional and use corev1.ConfigMapKeySelector (same for secret).

// Selects a key of a secret in the pod's namespace
// +optional
SecretKeyRef *SecretKeySelector `json:"secretKeyRef,omitempty" protobuf:"bytes,4,opt,name=secretKeyRef"`
ramondeklein marked this conversation as resolved.
Show resolved Hide resolved
}

// ConfigMapKeySelector selects a key from a ConfigMap.
// +structType=atomic
type ConfigMapKeySelector struct {
// The ConfigMap to select from.
corev1.LocalObjectReference `json:",inline" protobuf:"bytes,1,opt,name=localObjectReference"`
// The key to select.
Key string `json:"key" protobuf:"bytes,2,opt,name=key"`
}

// SecretKeySelector selects a key of a Secret.
// +structType=atomic
type SecretKeySelector struct {
// The name of the secret in the pod's namespace to select from.
corev1.LocalObjectReference `json:",inline" protobuf:"bytes,1,opt,name=localObjectReference"`
// The key of the secret to select from. Must be a valid secret key.
Key string `json:"key" protobuf:"bytes,2,opt,name=key"`
}
ramondeklein marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading