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

Handle showing errors for bad kots release blocking diff creation for newer releases #905

Merged
merged 6 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion kotsadm/pkg/handlers/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func SyncLicense(w http.ResponseWriter, r *http.Request) {
return
}

latestLicense, err := license.Sync(foundApp, syncLicenseRequest.LicenseData)
latestLicense, err := license.Sync(foundApp, syncLicenseRequest.LicenseData, true)
if err != nil {
logger.Error(err)
w.WriteHeader(500)
Expand Down
19 changes: 18 additions & 1 deletion kotsadm/pkg/handlers/rendered_contents.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ import (
"github.com/replicatedhq/kots/kotsadm/pkg/version"
)

type GetAppRenderedContentsResponse struct {
Files map[string]string `json:"files"`
}

type GetAppRenderedContentsErrorResponse struct {
Error string `json:"error"`
}

func GetAppRenderedContents(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Access-Control-Allow-Origin", "*")
w.Header().Set("Access-Control-Allow-Headers", "content-type, origin, accept, authorization")
Expand Down Expand Up @@ -88,6 +96,13 @@ func GetAppRenderedContents(w http.ResponseWriter, r *http.Request) {
if err != nil {
if ee, ok := err.(*exec.ExitError); ok {
err = fmt.Errorf("kustomize stderr: %q", string(ee.Stderr))
logger.Error(err)

JSON(w, 500, GetAppRenderedContentsErrorResponse{
Error: fmt.Sprintf("Failed to build release: %v", err),
})
return

}
logger.Error(err)
w.WriteHeader(500)
Expand All @@ -106,5 +121,7 @@ func GetAppRenderedContents(w http.ResponseWriter, r *http.Request) {
for filename, b := range archiveFiles {
decodedArchiveFiles[filename] = string(b)
}
JSON(w, 200, map[string]interface{}{"files": decodedArchiveFiles})
JSON(w, 200, GetAppRenderedContentsResponse{
Files: decodedArchiveFiles,
})
}
37 changes: 26 additions & 11 deletions kotsadm/pkg/license/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import (
"github.com/pkg/errors"
"github.com/replicatedhq/kots/kotsadm/pkg/app"
"github.com/replicatedhq/kots/kotsadm/pkg/kotsutil"
"github.com/replicatedhq/kots/kotsadm/pkg/logger"
"github.com/replicatedhq/kots/kotsadm/pkg/persistence"
"github.com/replicatedhq/kots/kotsadm/pkg/preflight"
"github.com/replicatedhq/kots/kotsadm/pkg/registry"
registrytypes "github.com/replicatedhq/kots/kotsadm/pkg/registry/types"
"github.com/replicatedhq/kots/kotsadm/pkg/render"
"github.com/replicatedhq/kots/kotsadm/pkg/version"
kotsv1beta1 "github.com/replicatedhq/kots/kotskinds/apis/kots/v1beta1"
Expand All @@ -22,7 +24,7 @@ import (
"k8s.io/client-go/kubernetes/scheme"
)

func Sync(a *app.App, licenseData string) (*kotsv1beta1.License, error) {
func Sync(a *app.App, licenseData string, failOnVersionCreate bool) (*kotsv1beta1.License, error) {
archiveDir, err := version.GetAppVersionArchive(a.ID, a.CurrentSequence)
if err != nil {
return nil, errors.Wrap(err, "failed to get latest app version")
Expand Down Expand Up @@ -88,21 +90,34 @@ func Sync(a *app.App, licenseData string) (*kotsv1beta1.License, error) {
return nil, errors.Wrap(err, "failed to get registry settings for app")
}

if err := render.RenderDir(archiveDir, a.ID, appSequence, registrySettings); err != nil {
return nil, errors.Wrap(err, "failed to render new version")
if err := createNewVersion(a, archiveDir, appSequence, registrySettings); err != nil {
// ignore error here to prevent a failure to render the current version
// preventing the end-user from updating the application
if failOnVersionCreate {
return nil, err
}
logger.Errorf("Failed to create new version from license sync: %v", err)
}
}

newSequence, err := version.CreateVersion(a.ID, archiveDir, "License Change", a.CurrentSequence)
if err != nil {
return nil, errors.Wrap(err, "failed to create new version")
}
return latestLicense, nil
}

if err := preflight.Run(a.ID, newSequence, archiveDir); err != nil {
return nil, errors.Wrap(err, "failed to run preflights")
}
func createNewVersion(a *app.App, archiveDir string, appSequence int64, registrySettings *registrytypes.RegistrySettings) error {
if err := render.RenderDir(archiveDir, a.ID, appSequence, registrySettings); err != nil {
return errors.Wrap(err, "failed to render new version")
}

return latestLicense, nil
newSequence, err := version.CreateVersion(a.ID, archiveDir, "License Change", a.CurrentSequence)
if err != nil {
return errors.Wrap(err, "failed to create new version")
}

if err := preflight.Run(a.ID, newSequence, archiveDir); err != nil {
return errors.Wrap(err, "failed to run preflights")
}

return nil
}

func GetCurrentLicenseString(a *app.App) (string, error) {
Expand Down
2 changes: 1 addition & 1 deletion kotsadm/pkg/updatechecker/updatechecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func CheckForUpdates(appID string, deploy bool) (int64, error) {
}

// sync license, this method is only called when online
_, err = license.Sync(a, "")
_, err = license.Sync(a, "", false)
if err != nil {
return 0, errors.Wrap(err, "failed to sync license")
}
Expand Down
102 changes: 71 additions & 31 deletions kotsadm/web/src/components/apps/AppVersionHistory.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class AppVersionHistory extends Component {
displayShowDetailsModal: false,
yamlErrorDetails: [],
deployView: false,
selectedSequence: ""
selectedSequence: "",
releaseWithErr: {}
}

componentDidMount() {
Expand Down Expand Up @@ -119,6 +120,13 @@ class AppVersionHistory extends Component {
});
}

toggleDiffErrModal = (release) => {
this.setState({
showDiffErrModal: !this.state.showDiffErrModal,
releaseWithErr: !this.state.showDiffErrModal ? release : {}
})
}

getVersionDiffSummary = version => {
if (!version.diffSummary || version.diffSummary === "") {
return null;
Expand All @@ -143,9 +151,9 @@ class AppVersionHistory extends Component {

renderYamlErrors = (yamlErrorsDetails, version) => {
return (
<div className="flex alignItems--center u-marginLeft--10">
<div className="flex alignItems--center">
<span className="icon error-small" />
<span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-marginLeft--5 u-color--red">{yamlErrorsDetails?.length} Invalid files </span>
<span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-marginLeft--5 u-color--red">{yamlErrorsDetails?.length} Invalid file{yamlErrorsDetails?.length !== 1 ? "s" : ""} </span>
<span className="replicated-link u-marginLeft--5 u-fontSize--small" onClick={() => this.toggleShowDetailsModal(yamlErrorsDetails, version.sequence)}> See details </span>
</div>
)
Expand All @@ -155,35 +163,45 @@ class AppVersionHistory extends Component {
const { app } = this.props;
const downstream = app.downstreams?.length && app.downstreams[0];
const diffSummary = this.getVersionDiffSummary(version);
const hasDiffSummaryError = version.diffSummaryError && version.diffSummaryError.length > 0;

return (
<div>
{diffSummary ?
(diffSummary.filesChanged > 0 ?
<div
className="DiffSummary u-cursor--pointer"
onClick={() => {
if (!downstream.gitops?.enabled) {
this.setState({
showDiffOverlay: true,
firstSequence: version.parentSequence - 1,
secondSequence: version.parentSequence
});
}
}}
>
<span className="files">{diffSummary.filesChanged} files changed </span>
<span className="lines-added">+{diffSummary.linesAdded} </span>
<span className="lines-removed">-{diffSummary.linesRemoved}</span>
</div>
:
<div className="DiffSummary">
<span className="files">No changes</span>
</div>
)
: <span>&nbsp;</span>}
if (hasDiffSummaryError) {
return (
<div className="flex flex1 alignItems--center u-marginRight--10">
<span className="u-fontSize--small u-fontWeight--medium u-lineHeight--normal u-color--dustyGray">Cannot generate diff <span className="replicated-link" onClick={() => this.toggleDiffErrModal(version)}>Why?</span></span>
</div>
);
);
} else {
return (
<div>
{diffSummary ?
(diffSummary.filesChanged > 0 ?
<div
className="DiffSummary u-cursor--pointer u-marginRight--10"
onClick={() => {
if (!downstream.gitops?.enabled) {
this.setState({
showDiffOverlay: true,
firstSequence: version.parentSequence - 1,
secondSequence: version.parentSequence
});
}
}}
>
<span className="files">{diffSummary.filesChanged} files changed </span>
<span className="lines-added">+{diffSummary.linesAdded} </span>
<span className="lines-removed">-{diffSummary.linesRemoved}</span>
</div>
:
<div className="DiffSummary">
<span className="files">No changes</span>
</div>
)
: <span>&nbsp;</span>}
</div>
);
}

}

renderVersionAction = (version, nothingToCommitDiff) => {
Expand Down Expand Up @@ -448,10 +466,13 @@ class AppVersionHistory extends Component {
});
}

hideDiffOverlay = () => {
hideDiffOverlay = (closeReleaseSelect) => {
this.setState({
showDiffOverlay: false
});
if (closeReleaseSelect) {
this.onCloseReleasesToDiff();
}
}

onSelectReleasesToDiff = () => {
Expand Down Expand Up @@ -1193,6 +1214,25 @@ class AppVersionHistory extends Component {
<button className="btn primary" onClick={this.hideDownstreamReleaseNotes}>Close</button>
</div>
</Modal>

<Modal
isOpen={this.state.showDiffErrModal}
onRequestClose={this.toggleDiffErrModal}
contentLabel="Unable to Get Diff"
ariaHideApp={false}
className="Modal MediumSize"
>
<div className="Modal-body">
<p className="u-fontSize--largest u-fontWeight--bold u-color--tuna u-lineHeight--normal u-marginBottom--10">Unable to generate a file diff for release</p>
<p className="u-fontSize--normal u-color--dustyGray u-lineHeight--normal u-marginBottom--20">The release with the <span className="u-fontWeight--bold">Upstream {this.state.releaseWithErr.title}, Sequence {this.state.releaseWithErr.sequence}</span> was unable to generate a files diff because the following error:</p>
<div className="error-block-wrapper u-marginBottom--30 flex flex1">
<span className="u-color--chestnut">{this.state.releaseWithErr.diffSummaryError}</span>
</div>
<div className="flex u-marginBottom--10">
<button className="btn primary" onClick={this.toggleDiffErrModal}>Ok, got it!</button>
</div>
</div>
</Modal>

{showUpdateCheckerModal &&
<UpdateCheckerModal
Expand Down
42 changes: 37 additions & 5 deletions kotsadm/web/src/components/watches/DownstreamWatchVersionDiff.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,20 @@ class DownstreamWatchVersionDiff extends React.Component {
constructor() {
super();
this.state = {
loadingFileTrees: false,
firstApplicationTree: {},
secondApplicationTree: {},
fileLoading: false,
fileLoadErr: false,
fileLoadErrMessage: "",
hasErrSettingDiff: false,
errSettingDiff: "",
failedSequence: undefined
};
}

fetchRenderedApplicationTree = (sequence, isFirst) => {
this.setState({ loadingFileTrees: true });
const url = `${window.env.API_ENDPOINT}/app/${this.props.slug}/sequence/${sequence}/renderedcontents`;
fetch(url, {
headers: {
Expand All @@ -31,13 +36,25 @@ class DownstreamWatchVersionDiff extends React.Component {
})
.then(res => res.json())
.then(async (files) => {
if (files.error) {
return this.setState({
hasErrSettingDiff: true,
errSettingDiff: files.error,
loadingFileTrees: false,
failedSequence: sequence
})
}
if (isFirst) {
this.setState({firstApplicationTree: files});
this.setState({ firstApplicationTree: files });
} else {
this.setState({secondApplicationTree: files});
this.setState({ secondApplicationTree: files });
}
if (this.state.firstApplicationTree.files && this.state.secondApplicationTree.files) {
this.setState({ loadingFileTrees: false });
}
})
.catch((err) => {
this.setState({ loadingFileTrees: false });
throw err;
});
}
Expand Down Expand Up @@ -73,22 +90,37 @@ class DownstreamWatchVersionDiff extends React.Component {

goBack = () => {
if (this.props.onBackClick) {
this.props.onBackClick();
this.props.onBackClick(true);
}
}

render() {
const { firstApplicationTree, secondApplicationTree } = this.state;
const { firstApplicationTree, secondApplicationTree, loadingFileTrees, hasErrSettingDiff, errSettingDiff, failedSequence } = this.state;
const { firstSequence, secondSequence } = this.props;

if (!firstApplicationTree.files || !secondApplicationTree.files) {
if (loadingFileTrees) {
return (
<div className="u-height--full u-width--full flex alignItems--center justifyContent--center u-marginTop--15">
<Loader size="60" />
</div>
);
}

if (hasErrSettingDiff) {
return (
<div className="u-height--full u-width--full flex-column alignItems--center justifyContent--center u-marginTop--15">
<p className="u-fontSize--largest u-fontWeight--bold u-color--tuna u-lineHeight--normal u-marginBottom--10">Unable to generate a file diff for the selected releases</p>
<p className="u-fontSize--normal u-color--dustyGray u-lineHeight--normal u-marginBottom--20">The release with the sequence <span className="u-fontWeight--bold">{failedSequence}</span> contains invalid YAML or config values and is unable to generate a diff. The full error is below.</p>
<div className="error-block-wrapper u-marginBottom--30 flex flex1">
<span className="u-color--chestnut">{errSettingDiff}</span>
</div>
<div className="flex u-marginBottom--10">
<button className="btn primary" onClick={() => this.goBack()}>Back to all versions</button>
</div>
</div>
)
}

const filesToInclude = [];
for (const filename in firstApplicationTree.files) {
if (firstApplicationTree.files[filename] === secondApplicationTree.files[filename]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ $cell-width: 140px;
.overlay {
user-select: none;
cursor: pointer;
a, button {
a, button, span {
pointer-events: none;
}
}
Expand Down
10 changes: 10 additions & 0 deletions kotsadm/web/src/scss/utilities/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ code {
padding: 10px;
}

.error-block-wrapper {
background-color: #FBECEA;
border-radius: 4px;
padding: 15px;
margin: 0 auto;
font-size: 14px;
line-height: 1.4;
font-weight: 500;
}

.errors {
display: inline-block;
color: #FFFFFF;
Expand Down