Skip to content

Commit

Permalink
Make Ansible Operator parameter case conversion optional (#3245)
Browse files Browse the repository at this point in the history
* Add parameter to turn off case conversion

* Add unit tests

* Add e2e test for case conversion

* Fix tests; add changelog fragment

* Add docs for snakeCaseParameters

* Reorganize code to avoid unnecessary loop

* fix alignment issue
  • Loading branch information
fabianvf committed Jul 16, 2020
1 parent 8599219 commit 4744d57
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 20 deletions.
10 changes: 10 additions & 0 deletions changelog/fragments/ansible-operator-optional-case.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
entries:
- description: >
The `snakeCaseParameters` option has been added to the `watches.yaml` for
Ansible-based Operators. This allows the user to configure whether parameters
in the resource spec are automatically converted from camelCase to snake_case.
The default is `true`, so there is no behavior change for existing operators,
but it can now be disabled.
kind: "addition"
breaking: false
45 changes: 28 additions & 17 deletions pkg/ansible/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,27 +131,29 @@ func New(watch watches.Watch) (Runner, error) {
}

return &runner{
Path: path,
cmdFunc: cmdFunc,
Vars: watch.Vars,
Finalizer: watch.Finalizer,
finalizerCmdFunc: finalizerCmdFunc,
GVK: watch.GroupVersionKind,
maxRunnerArtifacts: watch.MaxRunnerArtifacts,
ansibleVerbosity: watch.AnsibleVerbosity,
Path: path,
cmdFunc: cmdFunc,
Vars: watch.Vars,
Finalizer: watch.Finalizer,
finalizerCmdFunc: finalizerCmdFunc,
GVK: watch.GroupVersionKind,
maxRunnerArtifacts: watch.MaxRunnerArtifacts,
ansibleVerbosity: watch.AnsibleVerbosity,
snakeCaseParameters: watch.SnakeCaseParameters,
}, nil
}

// runner - implements the Runner interface for a GVK that's being watched.
type runner struct {
Path string // path on disk to a playbook or role depending on what cmdFunc expects
GVK schema.GroupVersionKind // GVK being watched that corresponds to the Path
Finalizer *watches.Finalizer
Vars map[string]interface{}
cmdFunc cmdFuncType // returns a Cmd that runs ansible-runner
finalizerCmdFunc cmdFuncType
maxRunnerArtifacts int
ansibleVerbosity int
Path string // path on disk to a playbook or role depending on what cmdFunc expects
GVK schema.GroupVersionKind // GVK being watched that corresponds to the Path
Finalizer *watches.Finalizer
Vars map[string]interface{}
cmdFunc cmdFuncType // returns a Cmd that runs ansible-runner
finalizerCmdFunc cmdFuncType
maxRunnerArtifacts int
ansibleVerbosity int
snakeCaseParameters bool
}

func (r *runner) Run(ident string, u *unstructured.Unstructured, kubeconfig string) (RunResult, error) {
Expand Down Expand Up @@ -307,7 +309,16 @@ func (r *runner) makeParameters(u *unstructured.Unstructured) map[string]interfa
spec = map[string]interface{}{}
}

parameters := paramconv.MapToSnake(spec)
parameters := map[string]interface{}{}

if r.snakeCaseParameters {
parameters = paramconv.MapToSnake(spec)
} else {
for k, v := range spec {
parameters[k] = v
}
}

parameters["meta"] = map[string]string{"namespace": u.GetNamespace(), "name": u.GetName()}

objKey := escapeAnsibleKey(fmt.Sprintf("_%v_%v", r.GVK.Group, strings.ToLower(r.GVK.Kind)))
Expand Down
11 changes: 10 additions & 1 deletion pkg/ansible/watches/watches.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type Watch struct {
ManageStatus bool `yaml:"manageStatus"`
WatchDependentResources bool `yaml:"watchDependentResources"`
WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"`
SnakeCaseParameters bool `yaml:"snakeCaseParameters"`
Selector metav1.LabelSelector `yaml:"selector"`

// Not configurable via watches.yaml
Expand All @@ -74,6 +75,7 @@ var (
manageStatusDefault = true
watchDependentResourcesDefault = true
watchClusterScopedResourcesDefault = false
snakeCaseParametersDefault = true
selectorDefault = metav1.LabelSelector{}

// these are overridden by cmdline flags
Expand Down Expand Up @@ -124,14 +126,14 @@ type alias struct {
ManageStatus *bool `yaml:"manageStatus,omitempty"`
WatchDependentResources *bool `yaml:"watchDependentResources,omitempty"`
WatchClusterScopedResources *bool `yaml:"watchClusterScopedResources,omitempty"`
SnakeCaseParameters *bool `yaml:"snakeCaseParameters"`
Blacklist []schema.GroupVersionKind `yaml:"blacklist,omitempty"`
Finalizer *Finalizer `yaml:"finalizer"`
Selector tempLabelSelector `yaml:"selector"`
}

// buildWatch will build Watch based on the values parsed from alias
func (w *Watch) setValuesFromAlias(tmp alias) error {

// by default, the operator will manage status and watch dependent resources
if tmp.ManageStatus == nil {
tmp.ManageStatus = &manageStatusDefault
Expand All @@ -151,10 +153,15 @@ func (w *Watch) setValuesFromAlias(tmp alias) error {
if tmp.WatchClusterScopedResources == nil {
tmp.WatchClusterScopedResources = &watchClusterScopedResourcesDefault
}

if tmp.Blacklist == nil {
tmp.Blacklist = blacklistDefault
}

if tmp.SnakeCaseParameters == nil {
tmp.SnakeCaseParameters = &snakeCaseParametersDefault
}

gvk := schema.GroupVersionKind{
Group: tmp.Group,
Version: tmp.Version,
Expand All @@ -175,6 +182,7 @@ func (w *Watch) setValuesFromAlias(tmp alias) error {
w.ReconcilePeriod = tmp.ReconcilePeriod.Duration
w.ManageStatus = *tmp.ManageStatus
w.WatchDependentResources = *tmp.WatchDependentResources
w.SnakeCaseParameters = *tmp.SnakeCaseParameters
w.WatchClusterScopedResources = *tmp.WatchClusterScopedResources
w.Finalizer = tmp.Finalizer
w.AnsibleVerbosity = getAnsibleVerbosity(gvk, ansibleVerbosityDefault)
Expand Down Expand Up @@ -302,6 +310,7 @@ func New(gvk schema.GroupVersionKind, role, playbook string, vars map[string]int
ManageStatus: manageStatusDefault,
WatchDependentResources: watchDependentResourcesDefault,
WatchClusterScopedResources: watchClusterScopedResourcesDefault,
SnakeCaseParameters: snakeCaseParametersDefault,
Finalizer: finalizer,
AnsibleVerbosity: ansibleVerbosityDefault,
Selector: selectorDefault,
Expand Down
6 changes: 6 additions & 0 deletions pkg/ansible/watches/watches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ func TestNew(t *testing.T) {
t.Fatalf("Unexpected watchDependentResources %v expected %v", watch.WatchDependentResources,
watchDependentResourcesDefault)
}
if watch.SnakeCaseParameters != snakeCaseParametersDefault {
t.Fatalf("Unexpected snakeCaseParameters %v expected %v", watch.SnakeCaseParameters,
snakeCaseParametersDefault)
}
if watch.WatchClusterScopedResources != watchClusterScopedResourcesDefault {
t.Fatalf("Unexpected watchClusterScopedResources %v expected %v",
watch.WatchClusterScopedResources, watchClusterScopedResourcesDefault)
Expand Down Expand Up @@ -143,6 +147,7 @@ func TestLoad(t *testing.T) {
ReconcilePeriod: twoSeconds,
WatchDependentResources: true,
WatchClusterScopedResources: false,
SnakeCaseParameters: true,
},
Watch{
GroupVersionKind: schema.GroupVersionKind{
Expand All @@ -153,6 +158,7 @@ func TestLoad(t *testing.T) {
Playbook: validTemplate.ValidPlaybook,
ManageStatus: true,
WatchDependentResources: true,
SnakeCaseParameters: false,
WatchClusterScopedResources: false,
Finalizer: &Finalizer{
Name: "finalizer.app.example.com",
Expand Down
22 changes: 22 additions & 0 deletions test/ansible/deploy/crds/test.example.com_casetest_crd.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: casetests.test.example.com
spec:
group: test.example.com
names:
kind: CaseTest
listKind: CaseTestList
plural: casetests
singular: casetest
scope: Namespaced
versions:
- name: v1alpha1
schema:
openAPIV3Schema:
type: object
x-kubernetes-preserve-unknown-fields: true
served: true
storage: true
subresources:
status: {}
25 changes: 25 additions & 0 deletions test/ansible/molecule/cluster/tasks/case_test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
- name: Create the test.example.com/v1alpha1.CaseTest
k8s:
state: present
definition:
apiVersion: test.example.com/v1alpha1
kind: CaseTest
metadata:
name: case-test
namespace: '{{ namespace }}'
spec:
camelCaseVar: "true"
wait: yes
wait_timeout: 300
wait_condition:
type: Running
reason: Successful
status: "True"
register: case_test

- name: Assert sentinel ConfigMap has been created for Molecule Test
assert:
that: cm.data.shouldBeCamel == 'true'
vars:
cm: "{{ q('k8s', api_version='v1', kind='ConfigMap', namespace=namespace, resource_name='case-test').0 }}"
17 changes: 17 additions & 0 deletions test/ansible/playbooks/case.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
- hosts: localhost
gather_facts: no
collections:
- community.kubernetes

tasks:
- name: Create configmap
k8s:
definition:
apiVersion: v1
kind: ConfigMap
metadata:
name: '{{ meta.name }}'
namespace: '{{ meta.namespace }}'
data:
shouldBeCamel: '{{ camelCaseVar | default("false") }}'
1 change: 0 additions & 1 deletion test/ansible/playbooks/secret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,3 @@
data:
'{{ item.key }}': '{{ item.value | b64decode }}'
with_dict: '{{ __secret.data }}'

5 changes: 5 additions & 0 deletions test/ansible/watches.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,8 @@
matchExpressions:
- {key: testLabel, operator: Exists, values: []}

- version: v1alpha1
group: test.example.com
kind: CaseTest
playbook: playbooks/case.yml
snakeCaseParameters: false
3 changes: 2 additions & 1 deletion website/content/en/docs/ansible/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ Resource spec field along to Ansible as extra
[variables](https://docs.ansible.com/ansible/2.5/user_guide/playbooks_variables.html#passing-variables-on-the-command-line).
The names of all variables in the spec field are converted to snake_case
by the operator before running ansible. For example, `serviceAccount` in
the spec becomes `service_account` in ansible.
the spec becomes `service_account` in ansible. You can disable this case conversion
by setting the `snakeCaseParameters` option to `false` in your `watches.yaml`.
It is recommended that you perform some type validation in Ansible on the
variables to ensure that your application is receiving expected input.

Expand Down
4 changes: 4 additions & 0 deletions website/content/en/docs/ansible/reference/watches.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ Some features can be overridden per resource via an annotation on that CR. The o
| Max Runner Artifacts | `maxRunnerArtifacts` | Manages the number of [artifact directories](https://ansible-runner.readthedocs.io/en/latest/intro.html#runner-artifacts-directory-hierarchy) that ansible runner will keep in the operator container for each individual resource. | ansible.operator-sdk/max-runner-artifacts | 20 | |
| Finalizer | `finalizer` | Sets a finalizer on the CR and maps a deletion event to a playbook or role | | | [finalizers](../finalizers)|
| Selector | `selector` | Identifies a set of objects based on their labels | | None Applied | [Labels and Selectors](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/)|
| Automatic Case Conversion | `snakeCaseParameters` | Determines whether to convert the CR spec from camelCase to snake_case before passing the contents to Ansible as extra_vars| | true | |


#### Example
```YaML
---
Expand All @@ -116,6 +119,7 @@ Some features can be overridden per resource via an annotation on that CR. The o
reconcilePeriod: 5s
manageStatus: False
watchDependentResources: False
snakeCaseParameters: False
finalizer:
name: finalizer.app.example.com
vars:
Expand Down

0 comments on commit 4744d57

Please sign in to comment.