Skip to content

Commit 98eaab2

Browse files
committed
pkg/repo: read remote KEP files from clone
The remote KEP loading code was using the local repo as its base and failing to find files. This updates the code to take two parameters, the path to the repo, and the path to the KEP in the repo. This allows `kepctl query --include-prs` to work (again? I'm not sure if it used to work and was broken during refactors) While I was here, I renamed ReadKEP to LoadLocalKEP (for parity with LoadLocalKEPs)
1 parent 944a19a commit 98eaab2

File tree

3 files changed

+53
-48
lines changed

3 files changed

+53
-48
lines changed

pkg/proposal/promote.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func Promote(opts *PromoteOpts) error {
5151

5252
logrus.Infof("Updating KEP %s/%s", opts.SIG, opts.Name)
5353

54-
p, err := r.ReadKEP(opts.SIG, opts.Name)
54+
p, err := r.LoadLocalKEP(opts.SIG, opts.Name)
5555
if err != nil {
5656
return fmt.Errorf("unable to load KEP for promotion: %s", err)
5757
}

pkg/repo/query.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (r *Repo) Query(opts *QueryOpts) ([]*api.Proposal, error) {
112112

113113
// Open PRs; existing KEPs with open PRs will be shown twice
114114
if opts.IncludePRs {
115-
prKeps, err := r.loadKEPPullRequests(sig)
115+
prKeps, err := r.LoadPullRequestKEPs(sig)
116116
if err != nil {
117117
logrus.Warnf("error searching for KEP PRs from %s: %s", sig, err)
118118
}

pkg/repo/repo.go

Lines changed: 51 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,8 @@ func (r *Repo) SetGitHubToken(tokenFile string) error {
185185
if err != nil {
186186
return err
187187
}
188-
189188
r.Token = strings.Trim(string(token), "\n\r")
190189
}
191-
192190
return nil
193191
}
194192

@@ -205,10 +203,7 @@ func (r *Repo) getProposalTemplate() ([]byte, error) {
205203
}
206204

207205
func (r *Repo) findLocalKEPMeta(sig string) ([]string, error) {
208-
sigPath := filepath.Join(
209-
r.ProposalPath,
210-
sig,
211-
)
206+
sigPath := filepath.Join(r.ProposalPath, sig)
212207

213208
keps := []string{}
214209

@@ -240,6 +235,10 @@ func (r *Repo) findLocalKEPMeta(sig string) ([]string, error) {
240235

241236
if info.Name() == ProposalMetadataFilename {
242237
logrus.Debugf("adding %s as KEP metadata", info.Name())
238+
path, err = filepath.Rel(r.BasePath, path)
239+
if err != nil {
240+
return err
241+
}
243242
keps = append(keps, path)
244243
return filepath.SkipDir
245244
}
@@ -269,27 +268,41 @@ func (r *Repo) LoadLocalKEPs(sig string) ([]*api.Proposal, error) {
269268
logrus.Debugf("loading the following local KEPs: %v", files)
270269

271270
var allKEPs []*api.Proposal
272-
for _, k := range files {
273-
if filepath.Ext(k) == ".yaml" {
274-
kep, err := r.loadKEPFromYaml(k)
275-
if err != nil {
276-
return nil, errors.Wrapf(
277-
err,
278-
"reading KEP %s from yaml",
279-
k,
280-
)
281-
}
282-
283-
allKEPs = append(allKEPs, kep)
271+
for _, kepYamlPath := range files {
272+
kep, err := r.loadKEPFromYaml(r.BasePath, kepYamlPath)
273+
if err != nil {
274+
return nil, errors.Wrapf(
275+
err,
276+
"reading KEP %s from yaml",
277+
kepYamlPath,
278+
)
284279
}
280+
281+
allKEPs = append(allKEPs, kep)
285282
}
286283

287284
logrus.Debugf("returning %d local KEPs", len(allKEPs))
288285

289286
return allKEPs, nil
290287
}
291288

292-
func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {
289+
func (r *Repo) LoadLocalKEP(sig, name string) (*api.Proposal, error) {
290+
kepPath := filepath.Join(
291+
ProposalPathStub,
292+
sig,
293+
name,
294+
ProposalMetadataFilename,
295+
)
296+
297+
_, err := os.Stat(kepPath)
298+
if err != nil {
299+
return nil, errors.Wrapf(err, "getting file info for %s", kepPath)
300+
}
301+
302+
return r.loadKEPFromYaml(r.BasePath, kepPath)
303+
}
304+
305+
func (r *Repo) LoadPullRequestKEPs(sig string) ([]*api.Proposal, error) {
293306
// Initialize github client
294307
logrus.Debugf("Initializing github client to load PRs for sig: %v", sig)
295308
var auth *http.Client
@@ -331,6 +344,7 @@ func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {
331344
}
332345
}
333346

347+
// Find KEP PRs for the given sig
334348
kepPRs := []*github.PullRequest{}
335349
sigLabel := strings.Replace(sig, "-", "/", 1)
336350
logrus.Debugf("Searching list of %v PRs for %v/%v with labels: [%v, %v]", len(r.allPRs), remoteOrg, remoteRepo, sigLabel, proposalLabel)
@@ -422,7 +436,13 @@ func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {
422436

423437
// read all these KEPs
424438
for k := range kepNames {
425-
kep, err := r.ReadKEP(sig, k)
439+
kepPath := filepath.Join(
440+
ProposalPathStub,
441+
sig,
442+
k,
443+
ProposalMetadataFilename,
444+
)
445+
kep, err := r.loadKEPFromYaml(r.gitRepo.Directory(), kepPath)
426446
if err != nil {
427447
logrus.Warnf("error reading KEP %v: %v", k, err)
428448
} else {
@@ -435,26 +455,13 @@ func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {
435455
return allKEPs, nil
436456
}
437457

438-
func (r *Repo) ReadKEP(sig, name string) (*api.Proposal, error) {
439-
kepPath := filepath.Join(
440-
r.ProposalPath,
441-
sig,
442-
name,
443-
ProposalMetadataFilename,
444-
)
445-
446-
_, err := os.Stat(kepPath)
447-
if err != nil {
448-
return nil, errors.Wrapf(err, "getting file info for %s", kepPath)
449-
}
450-
451-
return r.loadKEPFromYaml(kepPath)
452-
}
453-
454-
func (r *Repo) loadKEPFromYaml(kepPath string) (*api.Proposal, error) {
455-
b, err := ioutil.ReadFile(kepPath)
458+
// loadKEPFromYaml will return a Proposal from a kep.yaml at the given kepPath
459+
// within the given repoPath, or an error if the Proposal is invalid
460+
func (r *Repo) loadKEPFromYaml(repoPath, kepPath string) (*api.Proposal, error) {
461+
fullKEPPath := filepath.Join(repoPath, kepPath)
462+
b, err := ioutil.ReadFile(fullKEPPath)
456463
if err != nil {
457-
return nil, fmt.Errorf("unable to read KEP metadata: %s", err)
464+
return nil, fmt.Errorf("unable to read KEP metadata for %s: %w", fullKEPPath, err)
458465
}
459466

460467
var p api.Proposal
@@ -464,24 +471,22 @@ func (r *Repo) loadKEPFromYaml(kepPath string) (*api.Proposal, error) {
464471
}
465472

466473
p.Name = filepath.Base(filepath.Dir(kepPath))
474+
prrApprovalPath := filepath.Join(repoPath, ProposalPathStub, PRRApprovalPathStub)
467475

468476
// Read the PRR approval file and add any listed PRR approvers in there
469477
// to the PRR approvers list in the KEP. this is a hack while we transition
470478
// away from PRR approvers listed in kep.yaml
471479
handler := r.PRRHandler
472-
err = kepval.ValidatePRR(&p, handler, r.PRRApprovalPath)
480+
err = kepval.ValidatePRR(&p, handler, prrApprovalPath)
473481
if err != nil {
474482
logrus.Errorf(
475483
"%v",
476484
errors.Wrapf(err, "validating PRR for %s", p.Name),
477485
)
478486
} else {
479-
prrPath := filepath.Dir(kepPath)
480-
prrPath = filepath.Dir(prrPath)
481-
sig := filepath.Base(prrPath)
482-
prrPath = filepath.Join(
483-
filepath.Dir(prrPath),
484-
PRRApprovalPathStub,
487+
sig := filepath.Base(filepath.Dir(filepath.Dir(kepPath)))
488+
prrPath := filepath.Join(
489+
prrApprovalPath,
485490
sig,
486491
p.Number+".yaml",
487492
)

0 commit comments

Comments
 (0)