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

PTFE-687: action to sync two registries #88

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Abubakarr99
Copy link
Contributor

No description provided.

@Abubakarr99 Abubakarr99 requested a review from a team as a code owner July 27, 2023 16:27
Copy link

@gaspardmoindrot gaspardmoindrot left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tcarmet tcarmet left a comment

Choose a reason for hiding this comment

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

  • Add a test workflow that calls this action



runs:
using: "composite"
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making it a composite action, make it a docker one. Removing the need to ask users to use the proper container image and giving you more control over the environment.

Comment on lines +7 to +9
on:
workflow_dispatch:
```yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
on:
workflow_dispatch:
```yaml
```yaml
on:
workflow_dispatch:
# optionally show example with inputs.
inputs:
SOURCE_REPO:
required: true
IMAGE_NAME:
required: true

Then use those vars below.

Comment on lines +21 to +22
SRC_USERNAME: ${{ secrets.<src-username> }}
SRC_PASSWORD: ${{ secrets.<src-password> }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SRC_USERNAME: ${{ secrets.<src-username> }}
SRC_PASSWORD: ${{ secrets.<src-password> }}
SRC_USERNAME: ${{ secrets.REGISTRY_LOGIN }}
SRC_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }}

uses: scality/actions/registry-image-sync
with:
SOURCE_REPO: <repo-name> # Ex bert-e
IMAGE_NAME: <image-name> # Ex bert-e
Copy link
Contributor

Choose a reason for hiding this comment

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

You're making some parameters as required but not mentioning them here, feel free to setup default values for the destination.

required: true
TARGET_REGISTRY:
description: "the target registry"
required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
required: true
required: false
default: ghcr.io/scality

SRC_PASSWORD:
description: "the password in source registry"
DEST_USERNAME:
description: "the username in destination registry"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: "the username in destination registry"
description: "the username in destination registry"
default: ${{ github.actor }}

DEST_USERNAME:
description: "the username in destination registry"
DEST_PASSWORD:
description: "the password in destination registry"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: "the password in destination registry"
description: "the password in destination registry"
default: ${{ github.token }}

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.

3 participants