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

feat!: Replace sources with contentDirectories #18

Merged
merged 4 commits into from
Sep 28, 2022
Merged

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented Sep 9, 2022

Fixes #13.

Note
This will need a new major version.
Updates to the README can be more easily viewed here.

What does this change?

In this change we remove support for sources in the body of the riff-raff.yaml config, and replace it with a contentDirectories input to the Action.

This decoupling keeps the riff-raff.yaml config "valid". That is, we're able to copy/paste it to Riff-Raff's validation tool w/out first needing to remove sources.

It also means this action becomes compatible with auto-generated riff-raff.yaml files.

⚠️ BREAKING CHANGE: sources within riff-raff.yaml (either via the config or configPath input) is no longer supported

Configure the files to upload via the contentDirectories input, where contentDirectories is a mapping of package name -> artifact files.

From:

- uses: guardian/actions-riff-raff@v1
  with:
    app: my-application
    config: |
      stacks:
        - deploy
      regions:
        - eu-west-1
      allowedStages:
        - CODE
        - PROD
      deployments:
        static-site-assets:
          type: aws-s3
          sources:
            - static-site/dist # upload the `dist` directory
          parameters:
            bucket: aws-some-bucket
            cacheControl: private
            publicReadAcl: false
        api:
          type: autoscaling
          sources:
            - target/application.jar
          parameters:
            bucketSsmLookup: true

To:

- uses: guardian/actions-riff-raff@v2 # Note the version change
  with:
    app: my-application
    config: |
      stacks:
        - deploy
      regions:
        - eu-west-1
      allowedStages:
        - CODE
        - PROD
      deployments:
        static-site-assets: # <-- this is a package name
          type: aws-s3
          parameters:
            bucket: aws-some-bucket
            cacheControl: private
            publicReadAcl: false
        api: # <-- this is another package name
          type: autoscaling
          parameters:
            bucketSsmLookup: true
    contentDirectories: |
      static-site-assets:
        - static-site/dist
      api:
        - target/application.jar

How to test

See:

Copy link
Contributor

@nicl nicl left a comment

Choose a reason for hiding this comment

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

Have left some thoughts on testing especially - as would be keen for us to avoid the test-utils approach here. But in terms of feature-set this is great!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/test-utils.ts Outdated Show resolved Hide resolved
@akash1810 akash1810 force-pushed the aa-sources branch 2 times, most recently from 8b1a09a to f9c2505 Compare September 23, 2022 08:40
@akash1810 akash1810 changed the title feat: Add contentDirectories input feat!: Replace sources with contentDirectories Sep 23, 2022
@akash1810 akash1810 requested a review from nicl September 23, 2022 08:44
Fixes #13.

In this change we remove support for `sources` in the body of the `riff-raff.yaml` config,
and replace it with a `contentDirectories` input to the Action.

This decoupling keeps the `riff-raff.yaml` config "valid".
That is, we're able to copy/paste it into Riff-Raff's validation tool w/out first needing to remove `sources`.

It also means this action becomes compatible with auto-generated `riff-raff.yaml` files.

BREAKING CHANGE: `sources` within `riff-raff.yaml` (either via the `config` or `configPath` input) is no longer supported

Configure the files to upload via the `contentDirectories` input,
where `contentDirectories` is a mapping of package name -> artifact files.

From:

```yaml
- uses: guardian/actions-riff-raff@v1
  with:
    app: my-application
    config: |
      stacks:
        - deploy
      regions:
        - eu-west-1
      allowedStages:
        - CODE
        - PROD
      deployments:
        static-site-assets:
          type: aws-s3
          sources:
            - static-site/dist # upload the `dist` directory
          parameters:
            bucket: aws-some-bucket
            cacheControl: private
            publicReadAcl: false
        api:
          type: autoscaling
          sources:
            - target/application.jar
          parameters:
            bucketSsmLookup: true
```

To:

```yaml
- uses: guardian/actions-riff-raff@v2 # Note the version change
  with:
    app: my-application
    config: |
      stacks:
        - deploy
      regions:
        - eu-west-1
      allowedStages:
        - CODE
        - PROD
      deployments:
        static-site-assets: # <-- this is a package name
          type: aws-s3
          parameters:
            bucket: aws-some-bucket
            cacheControl: private
            publicReadAcl: false
        api: # <-- this is another package name
          type: autoscaling
          parameters:
            bucketSsmLookup: true
    contentDirectories: |
      static-site-assets:
        - static-site/dist
      api:
        - target/application.jar
```
@akash1810 akash1810 requested a review from a team September 27, 2022 08:24
Copy link
Contributor

@nicl nicl left a comment

Choose a reason for hiding this comment

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

LGTM! I'm tempted to attempt a slight shortening of the README as a follow up but this is a big improvement on things as it stands.

@akash1810 akash1810 merged commit bbee95d into main Sep 28, 2022
@akash1810 akash1810 deleted the aa-sources branch September 28, 2022 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support contentDirectory
2 participants