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

Use Remote Secret for storing image repository push token #67

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

mmorhun
Copy link
Collaborator

@mmorhun mmorhun commented Sep 26, 2023

This PRs makes push token stored in a RemoteSecret instead of just Secret. Such approach gives the following advantages:

  • Ability to propagate the token to BYOC and remote clusters
  • Makes Image Controller backup / restore friendly (avoiding backing up secrets directly)
  • Simplifies self healing implementation
  • Allows to have the same approach for push and pull tokens

@openshift-ci
Copy link

openshift-ci bot commented Sep 26, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

README.md Outdated Show resolved Hide resolved
api/v1alpha1/imagerepository_types.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tisutisu tisutisu left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -500,13 +491,16 @@ func (r *ImageRepositoryReconciler) EnsureRemotePullSecret(ctx context.Context,
return err
}

serviceAccountName := buildPipelineServiceAccountName
if isPull {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you using default SA for pull secret?

Copy link
Collaborator Author

@mmorhun mmorhun Oct 2, 2023

Choose a reason for hiding this comment

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

Because there is no build pipeline SA in that namespaces and also no guarantee that any other SA exists.
It was discussed during pull secret implementation.

},
},
},
},
},
},
}

if isPull {
Copy link
Contributor

Choose a reason for hiding this comment

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

also don't understand why this differs for pull & push secret

Copy link
Collaborator Author

@mmorhun mmorhun Oct 2, 2023

Choose a reason for hiding this comment

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

Because push token must be in current (build) namespace only (at least for current requirements), but pull token in all environments of the application.

@mmorhun mmorhun merged commit 4889322 into konflux-ci:main Oct 3, 2023
6 of 8 checks passed
@mmorhun mmorhun deleted the STONEBLD-1736 branch October 3, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants