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 for handle multiple backends #60

Merged
merged 7 commits into from
Mar 24, 2022

Conversation

mkuziemko
Copy link
Contributor

@mkuziemko mkuziemko commented Mar 10, 2022

Description

Changes proposed in this pull request:

Note: Modification was only made for Mattermost manifests.

Related issue(s)

@@ -464,7 +464,7 @@ spec:
- name: basicAuth
path: /yamls/basicAuth.yaml
container:
image: ghcr.io/capactio/pr/infra/merger:PR-428
image: ghcr.io/capactio/pr/infra/merger:20c169b
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
image: ghcr.io/capactio/pr/infra/merger:20c169b
image: ghcr.io/capactio/pr/infra/merger:20c169b

we cannot break this contract. All images under ghcr.io/capactio/pr/, need to stick to PR-{number} tag. How those images were pushed?

manifests/type/capactio/capact/validation/download.yaml Outdated Show resolved Hide resolved
@mkuziemko mkuziemko added the WIP Work in progress label Mar 15, 2022
@mkuziemko mkuziemko force-pushed the context_handling branch 4 times, most recently from 24a6ab9 to a260150 Compare March 15, 2022 15:18
@mkuziemko mkuziemko force-pushed the context_handling branch 2 times, most recently from 04a0e05 to 318c20e Compare March 21, 2022 12:38
@mkuziemko mkuziemko removed the WIP Work in progress label Mar 22, 2022
Copy link
Member

@mszostok mszostok left a comment

Choose a reason for hiding this comment

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

There where we use prepared inputs that are merged with our merger we shouldn't have problems, but in all places where we use capact-action: jinja2.template with "plain" input TypeInstance we should add unpackValue. It doesn't apply to input-parameters which are taken directly from user input and are not part of TypeInstance concept.

IMO, places where we miss unpack option:

  • - - name: create-es-ti
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: input-parameters
    from: "{{steps.terraform-apply.outputs.artifacts.additional}}"
    - name: configuration
    raw:
    data: |
    prefix: input
    - name: template
    raw:
    data: |
    version: <@ input.elasticsearch_version @>
    protocol: "https"
    host: "<@ input.endpoint @>"
    port: 443
    basicAuthSecretName: "<@ basicAuth.metadata.name @>"
  • - - name: render-postgresql
    capact-outputTypeInstances: # Defines which artifacts are output TypeInstances
    - name: postgresql
    from: render
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: input-parameters
    from: "{{steps.terraform-apply.outputs.artifacts.additional}}"
    - name: configuration
    raw:
    data: ""
    - name: template
    raw:
    data: |
    host: "<@ host @>"
    port: <@ port @>
    defaultDBName: "<@ defaultDBName @>"
    superuser:
    username: "<@ superuser.username @>"
    password: "<@ superuser.password @>"
  • - - name: render-rds
    capact-outputTypeInstances: # Defines which artifacts are output TypeInstances
    - name: rds-instance
    from: render
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: input-parameters
    from: "{{steps.terraform-apply.outputs.artifacts.additional}}"
    - name: configuration
    raw:
    data: ""
    - name: template
    raw:
    data: |
    identifier: "<@ identifier @>"
    availability_zone: "<@ availability_zone @>"
    class: "<@ class @>"
  • - - name: render-config
    capact-outputTypeInstances:
    - name: config
    from: render
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: input-parameters
    from: "{{steps.terraform-apply.outputs.artifacts.additional}}"
    - name: configuration
    raw:
    data: ""
    - name: template
    raw:
    data: |
    password: "<@ elasticache_auth_token @>"
    port: "<@ elasticache_port @>"
    host: "<@ elasticache_replication_group_primary_endpoint_address @>"
  • - - name: render-create-db-args
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: template
    raw:
    data: |
    name: concoursedb
    owner: "<@ name @>"
    - name: input-parameters
    from: "{{steps.create-user.outputs.artifacts.user}}"
    - name: configuration
    raw:
    data: |
  • - - name: render-create-db-args
    capact-action: jinja2.template
    arguments:
    artifacts:
    - name: template
    raw:
    data: |
    name: mattermost
    owner: "<@ name @>"
    - name: input-parameters
    from: "{{steps.create-user.outputs.artifacts.user}}"
    - name: configuration
    raw:
    data: |

But please double-check if all places really need it.

@mkuziemko mkuziemko merged commit c1a25f9 into capactio:main Mar 24, 2022
@mkuziemko mkuziemko deleted the context_handling branch March 24, 2022 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hub-manifests Relates to Hub manifests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants