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.

Sign up for GitHub

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

Jump to bottom

Add KEP for publishing packages in provisional state #843

Merged
merged 4 commits into from
Mar 22, 2019
Merged

Add KEP for publishing packages in provisional state #843

merged 4 commits into from
Mar 22, 2019

Conversation

hoegaarden
Copy link
Contributor

The purpose here is to get alignment on the Goals/Non-Goals and the rough proposal to get this KEP merged as provisional early.
Further (implementation) details and eventually the transition to implementable can be done with further PRs.

Supersedes: https://hackmd.io/PbH8KD09SFC0yn7ZhqVTRA?both

/cc @timothysc @sumitranr @Klaven @ixdy @ncdc @spiffxp @tpepper

/sig release
/area release-eng
/sig cluster-lifecycle

/kind kep

@k8s-ci-robot k8s-ci-robot added the sig/release Categorizes an issue or PR as relevant to SIG Release. label Feb 19, 2019
@k8s-ci-robot k8s-ci-robot added area/release-eng Issues or PRs related to the Release Engineering subproject sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/pm size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 19, 2019
timothysc
timothysc requested changes Feb 19, 2019
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

Minor comments, but let's setup some time to chat with some of the other infra folks to nail down some missing logistics.

/assign @ncdc @dims

keps/sig-release/20190219-publishing-packages.md Outdated Show resolved Hide resolved
keps/sig-release/20190219-publishing-packages.md Show resolved Hide resolved
keps/sig-release/20190219-publishing-packages.md Outdated

> [color=#ff0000] **TODO**

- *Risk*: We don't find a proper way to share secrets like the singing key*
Copy link
Member

Choose a reason for hiding this comment

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

@dims ^?

Copy link
Member

Choose a reason for hiding this comment

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

Currently our prow in test-infra does use secrets, so we should see if we can extend this to OpenPGP keys:
https://github.com/kyma-project/test-infra/blob/master/docs/prow/prow-secrets-management.md

Another thing to think about is how often we need to change the signing keys, see how openstack folks do this:
https://docs.openstack.org/infra/system-config/signing.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We know that the WG-k8s-infra are thinking about that. Still there is the risk that there isn't a fitting solution in time for when we want to implement this KEP.

Mitigation however might be:

  • we just don't sign packages for now, which is at least not worse than the current state
  • or we use git-crypt to manage secrets in git repos

and revisit that as soon as we have proper means to share and manage secrets?

@hoegaarden
Address comments:
40295eb 
- Unify some user stories / scenarios
- Add more detail about publishing different architectures
- Some typo fixes

Signed-off-by: Hannes Hörl <[email protected]>
@sumitranr
Copy link

/cc @listx

@k8s-ci-robot k8s-ci-robot requested a review from listx February 23, 2019 19:21
@hoegaarden
Copy link
Contributor Author

/assign @timothysc
/assign @spiffxp
/assign @tpepper

@hoegaarden
Copy link
Contributor Author

/cc @neolit123 as suggested by @timothysc

@k8s-ci-robot
Copy link
Contributor

@hoegaarden: GitHub didn't allow me to request PR reviews from the following users: as, suggested, by.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @neolit123 as suggested by @timothysc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@frapposelli
Copy link
Member

/cc @frapposelli

timothysc
timothysc approved these changes Mar 4, 2019
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

I think we've done enough bike-shedding, let's get this one in and iterate.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2019
@spiffxp
Copy link
Member

spiffxp commented Mar 4, 2019

/lgtm
I think this is provisional, I'd like to see more enumeration of needed infrastructure prior to implementable.

@hoegaarden
Copy link
Contributor Author

/hold
trying to get that to implementable before merge, as per discussion on slack.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2019
@hoegaarden hoegaarden mentioned this pull request Mar 4, 2019
Merged
@timothysc
Copy link
Member

I'd rather not hold this.
Let's just get this in and PR once you have some traction. Implementable needs to be met before feature freeze for 1.15.

@hoegaarden
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2019
timothysc
timothysc reviewed Mar 11, 2019
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 22, 2019
@hoegaarden
Copy link
Contributor Author

@spiffxp, can we merge that as provisional.

In the meantime I'd like to do some experiments re. promotion tool to get a better feel what we need if we

  • publish straight out of a bucket
  • publish via a self-hosted aplty or that like
  • publish via a SaaS packages publishing solution

I can then update the KEP with those findings and move it to implementable.

@timothysc
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 22, 2019
@spiffxp
Copy link
Member

spiffxp commented Mar 22, 2019

/lgtm

1 similar comment
@tpepper
Copy link
Member

tpepper commented Mar 22, 2019

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hoegaarden, spiffxp, timothysc, tpepper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2019
@k8s-ci-robot k8s-ci-robot merged commit f6ffb9b into kubernetes:master Mar 22, 2019
@justaugustus justaugustus mentioned this pull request Apr 30, 2020
Open
49 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ameukam ameukam ameukam left review comments

@listx listx listx left review comments

@frapposelli frapposelli frapposelli left review comments

@timothysc timothysc timothysc approved these changes

@dims dims dims left review comments

@ixdy ixdy Awaiting requested review from ixdy

@Klaven Klaven Awaiting requested review from Klaven

@ncdc ncdc Awaiting requested review from ncdc

@spiffxp spiffxp Awaiting requested review from spiffxp

@sumitranr sumitranr Awaiting requested review from sumitranr

@tpepper tpepper Awaiting requested review from tpepper

@neolit123 neolit123 Awaiting requested review from neolit123

Assignees

@ncdc ncdc

@dims dims

@spiffxp spiffxp

@timothysc timothysc

@tpepper tpepper

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.