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

PrivateKey with or without extra char as newline will be accepted #349

Merged
merged 1 commit into from
Jan 9, 2024
Merged

PrivateKey with or without extra char as newline will be accepted #349

merged 1 commit into from
Jan 9, 2024

Conversation

kumaritanushree
Copy link
Contributor

@kumaritanushree kumaritanushree commented Jan 5, 2024 •
edited
Loading

fix #350

100mik
100mik approved these changes Jan 5, 2024
View reviewed changes
Copy link
Contributor

@100mik 100mik left a comment

Choose a reason for hiding this comment

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

Verified that these changes work! :GTM 🚀

@joaopapereira
Copy link
Member

Is there any kind of test that we can do to ensure that we do not break this in the future? If not we should add a comment to that line to explain why that new line is there

@rohitagg2020
Copy link
Contributor

LGTM

praveenrewar
praveenrewar reviewed Jan 8, 2024
View reviewed changes
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

+1 to what Joao said.
We already seem to be adding a newline while adding gitCreds (on line 141), should we check if there are other places where a newline needs to be added (maybe as part of a separate issue)?

@kumaritanushree
Copy link
Contributor Author

kumaritanushree commented Jan 9, 2024 •
edited
Loading

Is there any kind of test that we can do to ensure that we do not break this in the future? If not we should add a comment to that line to explain why that new line is there

Added comment as of now as to add e2e test we will required to maintain dummy private repo and private key to access that. I think comment should be fine.

Zebradil
Zebradil reviewed Jan 9, 2024
View reviewed changes
pkg/vendir/fetch/git/git.go Outdated
Comment on lines 96 to 97
// Adding newlinne as git does not support private-key without newline as last character.
// Now vendir will work with both with/without newline in private-key.
Copy link
Member

Choose a reason for hiding this comment

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

At first, I noticed a typo (newlinne), but decided to expand the suggestion. How about this?

Suggested change
// Adding newlinne as git does not support private-key without newline as last character.
// Now vendir will work with both with/without newline in private-key.
// Ensure the private key ends with a newline character, as git requires it to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

joaopapereira
joaopapereira approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

Making a test for this would have been great but we would have needed to create private repositories and in some way share the keys which is not great and that would be an extra issue for local development. Also thinking a little bit more we could try to do some sort of unit tests for this but we might need to do a major refactor of this struct, which for now I think we should hold of.
Due to this I am ok with the comment added, and the PR is ready for merge when tests are green

@joaopapereira joaopapereira merged commit e4babdd into develop Jan 9, 2024
4 checks passed
@joaopapereira joaopapereira deleted the enhance-sshKey-acceptance branch January 9, 2024 17:43
renovate bot added a commit to mykso/myks that referenced this pull request Jan 22, 2024
@renovate
fix(deps): update module carvel.dev/vendir to v0.39.0 (#225)
3a99a5d 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [carvel.dev/vendir](https://togithub.com/carvel-dev/vendir) |
`v0.38.0` -> `v0.39.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/carvel.dev%2fvendir/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/carvel.dev%2fvendir/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/carvel.dev%2fvendir/v0.38.0/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/carvel.dev%2fvendir/v0.38.0/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>carvel-dev/vendir (carvel.dev/vendir)</summary>

###
[`v0.39.0`](https://togithub.com/carvel-dev/vendir/releases/tag/v0.39.0)

[Compare
Source](https://togithub.com/carvel-dev/vendir/compare/v0.38.0...v0.39.0)

<details>

<summary><h2>Installation and signature verification</h2></summary>

##### Installation
##### By downloading binary from the release

For instance, if you are using Linux on an AMD64 architecture:

```shell

### Download the binary
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/vendir-linux-amd64

### Move the binary in to your PATH
mv vendir-linux-amd64 /usr/local/bin/vendir

### Make the binary executable
chmod +x /usr/local/bin/vendir
```

##### Via Homebrew (macOS or Linux)

```shell
$ brew tap carvel-dev/carvel
$ brew install vendir
$ vendir version  
```

##### Verify checksums file signature

Install cosign on your system
https://docs.sigstore.dev/system_config/installation/

The checksums file provided within the artifacts attached to this
release is signed using
[Cosign](https://docs.sigstore.dev/cosign/overview/) with GitHub OIDC.
To validate the signature of this file, run the following commands:

```shell

### Download the checksums file, certificate and signature
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.pem
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.sig

### Verify the checksums file
cosign verify-blob checksums.txt \
  --certificate checksums.txt.pem \
  --signature checksums.txt.sig \
  --certificate-identity-regexp=https://github.com/carvel-dev \
  --certificate-oidc-issuer=https://token.actions.githubusercontent.com
```

##### Verify binary integrity

To verify the integrity of the downloaded binary, you can utilize the
checksums file after having validated its signature.

```shell

### Verify the binary using the checksums file
sha256sum -c checksums.txt --ignore-missing
```

</details>

### ✨ What's new
* fix grammar in README by
@&#8203;vtrent[carvel-dev/vendir#324
* Added changes to sign artifacts by
@&#8203;kumaritanushr[carvel-dev/vendir#339
* Simplify gitignore and make sure all binaries are accounted for by
@&#8203;100m[carvel-dev/vendir#351
* PrivateKey with or without extra char as newline will be accepted by
@&#8203;kumaritanushr[carvel-dev/vendir#349
* Fix race condition when running multiple vendir from the same
directory by
@&#8203;Zebrad[carvel-dev/vendir#345
* Refactor lazy sync code by
@&#8203;Zebrad[carvel-dev/vendir#340
* Fix: updated setup cosign step in release process by
@&#8203;kumaritanushr[carvel-dev/vendir#352
* updated release to have installation and verification steps included
in release notes by
@&#8203;kumaritanushr[carvel-dev/vendir#354

#### New Contributors
* @&#8203;vtrenton made their first
contributi[carvel-dev/vendir#324
* @&#8203;100mik made their first
contributi[carvel-dev/vendir#351

**Full Changelog**:
carvel-dev/vendir@v0.38.0...v0.39.0

### 📂 Files Checksum

012531a2f1a2de8bc89f1623edfc40a7ac5aee421fe609085278fb9e287f1cdf
./vendir-linux-arm64
20b71cc25dc3fea31edf9667c92a05167f713935f854882159736443c2f7a0e6
./vendir-windows-amd64.exe
90ae82718c1072831f3097bdb031d5a897cc9f2f8334e2e1d7f35e35d0abd84f
./vendir-darwin-amd64
91ecf04ad5cdfa0f8839dc1430da7a4da665f7cb88c64c0c72202f6db261e651
./vendir-darwin-arm64
feb2836153508adfb6fd33c127e466c9ce26577678e93a252be2fec445f4501f
./vendir-linux-amd64

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log [here](https://developer.mend.io/github/mykso/myks).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to mykso/myks that referenced this pull request Jan 22, 2024
@renovate
chore(deps): update dependency carvel-dev/vendir to v0.39.0 (#224)
492fb54 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [carvel-dev/vendir](https://togithub.com/carvel-dev/vendir) | minor |
`v0.38.0` -> `v0.39.0` |

---

### Release Notes

<details>
<summary>carvel-dev/vendir (carvel-dev/vendir)</summary>

###
[`v0.39.0`](https://togithub.com/carvel-dev/vendir/releases/tag/v0.39.0)

[Compare
Source](https://togithub.com/carvel-dev/vendir/compare/v0.38.0...v0.39.0)

<details>

<summary><h2>Installation and signature verification</h2></summary>

##### Installation
##### By downloading binary from the release

For instance, if you are using Linux on an AMD64 architecture:

```shell

### Download the binary
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/vendir-linux-amd64

### Move the binary in to your PATH
mv vendir-linux-amd64 /usr/local/bin/vendir

### Make the binary executable
chmod +x /usr/local/bin/vendir
```

##### Via Homebrew (macOS or Linux)

```shell
$ brew tap carvel-dev/carvel
$ brew install vendir
$ vendir version  
```

##### Verify checksums file signature

Install cosign on your system
https://docs.sigstore.dev/system_config/installation/

The checksums file provided within the artifacts attached to this
release is signed using
[Cosign](https://docs.sigstore.dev/cosign/overview/) with GitHub OIDC.
To validate the signature of this file, run the following commands:

```shell

### Download the checksums file, certificate and signature
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.pem
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.sig

### Verify the checksums file
cosign verify-blob checksums.txt \
  --certificate checksums.txt.pem \
  --signature checksums.txt.sig \
  --certificate-identity-regexp=https://github.com/carvel-dev \
  --certificate-oidc-issuer=https://token.actions.githubusercontent.com
```

##### Verify binary integrity

To verify the integrity of the downloaded binary, you can utilize the
checksums file after having validated its signature.

```shell

### Verify the binary using the checksums file
sha256sum -c checksums.txt --ignore-missing
```

</details>

### ✨ What's new
* fix grammar in README by
@&#8203;vtrent[carvel-dev/vendir#324
* Added changes to sign artifacts by
@&#8203;kumaritanushr[carvel-dev/vendir#339
* Simplify gitignore and make sure all binaries are accounted for by
@&#8203;100m[carvel-dev/vendir#351
* PrivateKey with or without extra char as newline will be accepted by
@&#8203;kumaritanushr[carvel-dev/vendir#349
* Fix race condition when running multiple vendir from the same
directory by
@&#8203;Zebrad[carvel-dev/vendir#345
* Refactor lazy sync code by
@&#8203;Zebrad[carvel-dev/vendir#340
* Fix: updated setup cosign step in release process by
@&#8203;kumaritanushr[carvel-dev/vendir#352
* updated release to have installation and verification steps included
in release notes by
@&#8203;kumaritanushr[carvel-dev/vendir#354

#### New Contributors
* @&#8203;vtrenton made their first
contributi[carvel-dev/vendir#324
* @&#8203;100mik made their first
contributi[carvel-dev/vendir#351

**Full Changelog**:
carvel-dev/vendir@v0.38.0...v0.39.0

### 📂 Files Checksum

012531a2f1a2de8bc89f1623edfc40a7ac5aee421fe609085278fb9e287f1cdf
./vendir-linux-arm64
20b71cc25dc3fea31edf9667c92a05167f713935f854882159736443c2f7a0e6
./vendir-windows-amd64.exe
90ae82718c1072831f3097bdb031d5a897cc9f2f8334e2e1d7f35e35d0abd84f
./vendir-darwin-amd64
91ecf04ad5cdfa0f8839dc1430da7a4da665f7cb88c64c0c72202f6db261e651
./vendir-darwin-arm64
feb2836153508adfb6fd33c127e466c9ce26577678e93a252be2fec445f4501f
./vendir-linux-amd64

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log [here](https://developer.mend.io/github/mykso/myks).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zebradil Zebradil Zebradil left review comments

@praveenrewar praveenrewar praveenrewar left review comments

@joaopapereira joaopapereira joaopapereira approved these changes

@100mik 100mik 100mik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Carvel
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vendir is failing to fetch content if private-key does not have newline at the end
6 participants
@kumaritanushree @joaopapereira @rohitagg2020 @Zebradil @praveenrewar @100mik