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

Validate: add ability to expand src validate commands for future health check functionality #921

Merged
merged 12 commits into from
Jan 13, 2023

Conversation

jdpleiness
Copy link
Contributor

@jdpleiness jdpleiness commented Jan 12, 2023

To support expansion of the src validate command with new health check features, the validate command needs to be broken down into subcommands.

This PR moves the current src validate command to src validate install as this command validates a Sourcegraph installation is setup and working correctly. Additional changes and features are listed below:

  • Switch to using existing internal/api package for GraphQL queries
  • Remove create first admin feature that allowed the validate command to create a Sourcegraph admin user
  • Refactor configuration layout to allow easier support for multiple code hosts in the future. src validate currently only supports GitHub, but adding additional code hosts such as GitLab is now much easier.
  • Add visual feedback to CLI for success or failure of steps
  • Add cleanup of insight testing as mentioned here
  • Move GitHub token from config file to an environmental variable in alignment with other account tokens

validate-install

validate-install-yaml

validate-install-json

TODO:

  • Update CHANGELOG.md

Test plan

Locally tested changes using latest Sourcegraph Helm install. Tested default configuration if no YAML is provided, YAML configuration provided, and JSON configuration provided.

  • ran go test ./... - all tests passing
  • ran staticcheck ./... - no issues found in new code
Example YAML file used:
externalService:
  kind: GITHUB
  displayName: srcgraph-test
  deleteWhenDone: true
  maxRetries: 5
  retryTimeoutSeconds: 5
  config:
    gitHub:
      url: https://github.com
      orgs: []
      repos:
        - sourcegraph-testing/zap

searchQuery:
  - repo:^github.com/sourcegraph-testing/zap$ test
  - repo:^github.com/sourcegraph-testing/zap$@v1.14.1 test

insight:
  title: "Javascript to Typescript migration"
  dataSeries:
    [ {
      "query": "lang:javascript",
      "label": "javascript",
      "repositoryScope": [
        "github.com/sourcegraph/sourcegraph"
      ],
      "lineColor": "#6495ED",
      "timeScopeUnit": "MONTH",
      "timeScopeValue": 1
    },
      {
        "query": "lang:typescript",
        "label": "typescript",
        "lineColor": "#DE3163",
        "repositoryScope": [
          "github.com/sourcegraph/sourcegraph"
        ],
        "timeScopeUnit": "MONTH",
        "timeScopeValue": 1
      }
    ]
Example JSON file used:
{
  "externalService": {
    "kind": "GITHUB",
    "displayName": "srcgraph-test",
    "deleteWhenDone": true,
    "maxRetries": 5,
    "retryTimeoutSeconds": 5,
    "config": {
      "gitHub": {
        "url": "https://github.com",
        "orgs": [],
        "repos": [
          "sourcegraph-testing/zap"
        ]
      }
    }
  },

  "searchQuery": [
    "repo:^github.com/sourcegraph-testing/zap$ test",
    "repo:^github.com/sourcegraph-testing/zap$@v1.14.1 test"
  ],

  "insight": {
    "title": "Javascript to Typescript migration",
    "dataSeries": [
      {
        "query": "lang:javascript",
        "label": "javascript",
        "repositoryScope": [
          "github.com/sourcegraph/sourcegraph"
        ],
        "lineColor": "#6495ED",
        "timeScopeUnit": "MONTH",
        "timeScopeValue": 1
      },
      {
        "query": "lang:typescript",
        "label": "typescript",
        "lineColor": "#DE3163",
        "repositoryScope": [
          "github.com/sourcegraph/sourcegraph"
        ],
        "timeScopeUnit": "MONTH",
        "timeScopeValue": 1
      }
    ]
  }

}

To support expansion of the validate command, the current validate
functionality will need to be moved to a new subcommand.

* Use existing internal api package for graphql requests
* Remove create first admin features
* Refactor configuration structs to support multiple code hosts in future
* Add cool emoji to output
* Add insight test cleanup
Github access token is needed for new validate command. Add token to config.
* Move 'src validate' functionality into 'srv validate install' commmand to allow for
more validate subcommands.
@jdpleiness jdpleiness self-assigned this Jan 12, 2023
@jdpleiness jdpleiness marked this pull request as ready for review January 12, 2023 19:07
jdpleiness added a commit to sourcegraph/sourcegraph-public-snapshot that referenced this pull request Jan 12, 2023
Update YAML and JSON configuration specifications to reflect changes in [#921](sourcegraph/src-cli#921)
cmd/src/validate_install.go Show resolved Hide resolved
* An auth token is required for code hosts when using `src validate install`, warn user and exit if not set.
Copy link

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

I like how nice the output looks!
Idea: In addition to the current output, I think it'd be nice to show the services each task are related to, so that if one of test fails, the user will know what to look into?

// DefaultConfig returns a default configuration to be used for testing.
func DefaultConfig() *ValidationSpec {
return &ValidationSpec{
SearchQuery: []string{"repo:^github\\.com/gorilla/mux$ Router", "repo:^github\\.com/gorilla/mux$@v1.8.0 Router"},

Choose a reason for hiding this comment

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

This is great! On top of performing an indexed search and unindexed search, do you think it'd be helpful to add a default symbol search (confirm connection to the symbol service)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, thats a good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added a symbol search to the defaults, should be gtg.

Copy link

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

I just noticed https://github.com/gorilla/mux has been archive so the search queries will no longer work. We might want to use another repo for default

* Change default queries to use sourcegraph/src-cli repo instead of gorilla/mux
@jdpleiness
Copy link
Contributor Author

I've swapped out gorilla/mux for sourcegraph/cli in the default params.

Example of new changes:
Screenshot 2023-01-13 at 11 52 08 AM

Copy link

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

yay thanks for the quick update! LGTM!

@jdpleiness jdpleiness merged commit 0ffe3d3 into main Jan 13, 2023
@jdpleiness jdpleiness deleted the jdp/validate-install branch January 13, 2023 18:18
jdpleiness added a commit to sourcegraph/sourcegraph-public-snapshot that referenced this pull request Jan 13, 2023
…command (#46397)

* Update YAML and JSON config specs

Update YAML and JSON configuration specifications to reflect changes in [#921](sourcegraph/src-cli#921)

* Update examples with new `src validate` command

* Fix formatting issues

* Fix spelling
Copy link
Contributor

@DaedalusG DaedalusG left a comment

Choose a reason for hiding this comment

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

Late to the party but this looks great!

scjohns pushed a commit that referenced this pull request Apr 24, 2023
…alth check functionality (#921)

* Refactor validate functionality into pkg

To support expansion of the validate command, the current validate
functionality will need to be moved to a new subcommand.

* Use existing internal api package for graphql requests
* Remove create first admin features
* Refactor configuration structs to support multiple code hosts in future
* Add cool emoji to output
* Add insight test cleanup

* Add env vars for Github access token

Github access token is needed for new validate command. Add token to config.

* Add 'src validate install' command

* Move 'src validate' functionality into 'srv validate install' commmand to allow for
more validate subcommands.

* Change validation API to be more specific

* Change SRC_GITHUB_TOKEN to only be read if needed for command

* Update CHANGELOG.md

* Add warning if SRC_GITHUB_TOKEN isn't set

* An auth token is required for code hosts when using `src validate install`, warn user and exit if not set.

* Remove unused deps

* Separate tasks being performed

* Add symbol search to default queries

* Change default queries to use sourcegraph/src-cli repo instead of gorilla/mux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants