Open guisea opened 7 months ago
@anbraten here is that Vultr provider I mentioned. Straightened out the issue with SSH Keys it had.
Built and tested a container locally and machines are provisioned and destroyed at Vultr on demand.
Ahhh, yes we should not leak the token.
I thought I axed all the debug info I sprinkled through it.
I will review these suggestions when I have some time tomorrow ;)
Regards
Aaron
On Wed, 7 Feb 2024 at 9:31 PM, Anbraten @.***> wrote:
@.**** commented on this pull request.
In providers/vultr/provider.go https://github.com/woodpecker-ci/autoscaler/pull/88#discussion_r1470719896 :
- region string
- enableIPv6 bool
- name string
- client *govultr.Client +}
+func New(c cli.Context, config config.Config) (engine.Provider, error) {
- p := &Provider{
- name: "vultr",
- region: c.String("vultr-region"),
- plan: c.String("vultr-plan"),
- image: c.String("vultr-image"),
- enableIPv6: c.Bool("vultr-public-ipv6-enable"),
- config: config,
- }
- fmt.Fprintf(os.Stdout, "API Token is: %s", c.String("vultr-api-token"))
We should not leak the token 😉 ⬇️ Suggested change
- fmt.Fprintf(os.Stdout, "API Token is: %s", c.String("vultr-api-token"))
In providers/vultr/provider.go https://github.com/woodpecker-ci/autoscaler/pull/88#discussion_r1470722837 :
- if err != nil {
- return fmt.Errorf("%s: RenderUserDataTemplate: %w", p.name, err)
- }
- image := -1
- osList, , , err := p.client.OS.List(ctx, &govultr.ListOptions{})
- if err != nil {
- return fmt.Errorf("%s: OS.List: %w", p.name, err)
- }
- for _, osS := range osList {
- if osS.Name == p.image {
- image = osS.ID
- break
- }
- }
⬇️ Suggested change
- +if image == -1 {
- return fmt.Errorf("%s: DeployAgent: no image found for %s", p.name, p.image) +}
In providers/vultr/provider.go https://github.com/woodpecker-ci/autoscaler/pull/88#discussion_r1470783520 :
- if !ok {
- continue
- }
- p.sshKeys = append(p.sshKeys, fingerprint)
- return nil
- }
- // if there were no matches but the account has at least
- // one keypair already created we will select the first
- // in the list.
- if len(res) > 0 {
- p.sshKeys = append(p.sshKeys, res[0].ID)
- return nil
- }
- // "No matching keys"
The error is descriptive enough I guess ⬇️ Suggested change
- // "No matching keys"
— Reply to this email directly, view it on GitHub https://github.com/woodpecker-ci/autoscaler/pull/88#pullrequestreview-1850427200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7Q3YCLCOZL7DDHF23NQCDYSM3X7AVCNFSM6AAAAABCQMOJPWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJQGQZDOMRQGA . You are receiving this because you authored the thread.Message ID: @.***>
@anbraten Suggestions have been incorporated and changes merged from main
Hey @anbraten is there anything else needed here?
@guisea can you rebase to the main branch please?
@xoxys Rebased and fixed up some linting/formatting errors.
Thanks. Do we know if Vultr is affected by this https://github.com/woodpecker-ci/autoscaler/issues/91? I can't find anything in the metadata api docs and don't have an account to test it.
From what I could see only instance data is available per the docs. I believe user data is provided as file so token is only exposed there.
On Mar 22, 2024 at 9:15 AM, <Robert Kaussow @.***)> wrote:
Thanks. Do we know if Vultr is affected by this #91 (https://github.com/woodpecker-ci/autoscaler/issues/91)? I can't find anything in the metadata api docs (https://www.vultr.com/metadata/) and don't have an account to test it.
— Reply to this email directly, view it on GitHub (https://github.com/woodpecker-ci/autoscaler/pull/88#issuecomment-2013642824), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AA7Q3YG7OIUW47HPP4KBJG3YZM5UXAVCNFSM6AAAAABCQMOJPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJTGY2DEOBSGQ). You are receiving this because you were mentioned.Message ID: @.***>
This is an implementation of a provider for Vultr cloud service.